diff --git a/changelog/unreleased/enhancement-add-configurable-group-members-patch-limit.md b/changelog/unreleased/enhancement-add-configurable-group-members-patch-limit.md new file mode 100644 index 00000000000..261fb3757fe --- /dev/null +++ b/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 diff --git a/services/graph/pkg/config/config.go b/services/graph/pkg/config/config.go index 79d18268a9a..5fa9843a319 100644 --- a/services/graph/pkg/config/config.go +++ b/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."` diff --git a/services/graph/pkg/config/defaults/defaultconfig.go b/services/graph/pkg/config/defaults/defaultconfig.go index 84c0fc94936..5f4d15cdb67 100644 --- a/services/graph/pkg/config/defaults/defaultconfig.go +++ b/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", diff --git a/services/graph/pkg/service/v0/groups.go b/services/graph/pkg/service/v0/groups.go index 760105f1de3..e17584b8668 100644 --- a/services/graph/pkg/service/v0/groups.go +++ b/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)) diff --git a/services/graph/pkg/service/v0/groups_test.go b/services/graph/pkg/service/v0/groups_test.go index 02cbe9d71fc..d5ada43d196 100644 --- a/services/graph/pkg/service/v0/groups_test.go +++ b/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 member@odata.bind values")) Expect(rr.Code).To(Equal(http.StatusBadRequest)) })