Graph test coverage (#5098)

* First iteration of a groups test * Do not render a result in case of an error * Generate mocks for identity.Backend * Export listResponse (-> ListResponse) * Add unit tests for GetGroups * Finish GetGroups unit tests * Increase test coverage * Increase test coverage * Increase test coverage * Add missing mocks * Fix return codes * Fix error messages
2022-11-23 14:01:29 +01:00
parent 1b764d2142
commit 7c2961e18a
8 changed files with 774 additions and 13 deletions
@@ -29,6 +29,7 @@ ci-go-generate: $(MOCKERY) # CI runs ci-node-generate automatically before this
 	$(MOCKERY) --dir pkg/service/v0 --case underscore --name HTTPClient
 	$(MOCKERY) --dir pkg/service/v0 --case underscore --name Publisher
 	$(MOCKERY) --dir pkg/service/v0 --case underscore --name Permissions
+	$(MOCKERY) --dir pkg/identity --output pkg/identity/mocks --case underscore --name Backend
 	$(MOCKERY) --srcpkg github.com/go-ldap/ldap/v3 --case underscore --filename ldapclient.go --name Client


@@ -0,0 +1,273 @@
+// Code generated by mockery v2.14.1. DO NOT EDIT.
+
+package mocks
+
+import (
+	context "context"
+
+	libregraph "github.com/owncloud/libre-graph-api-go"
+
+	mock "github.com/stretchr/testify/mock"
+
+	url "net/url"
+)
+
+// Backend is an autogenerated mock type for the Backend type
+type Backend struct {
+	mock.Mock
+}
+
+// AddMembersToGroup provides a mock function with given fields: ctx, groupID, memberID
+func (_m *Backend) AddMembersToGroup(ctx context.Context, groupID string, memberID []string) error {
+	ret := _m.Called(ctx, groupID, memberID)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(context.Context, string, []string) error); ok {
+		r0 = rf(ctx, groupID, memberID)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+// CreateGroup provides a mock function with given fields: ctx, group
+func (_m *Backend) CreateGroup(ctx context.Context, group libregraph.Group) (*libregraph.Group, error) {
+	ret := _m.Called(ctx, group)
+
+	var r0 *libregraph.Group
+	if rf, ok := ret.Get(0).(func(context.Context, libregraph.Group) *libregraph.Group); ok {
+		r0 = rf(ctx, group)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*libregraph.Group)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(context.Context, libregraph.Group) error); ok {
+		r1 = rf(ctx, group)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// CreateUser provides a mock function with given fields: ctx, user
+func (_m *Backend) CreateUser(ctx context.Context, user libregraph.User) (*libregraph.User, error) {
+	ret := _m.Called(ctx, user)
+
+	var r0 *libregraph.User
+	if rf, ok := ret.Get(0).(func(context.Context, libregraph.User) *libregraph.User); ok {
+		r0 = rf(ctx, user)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*libregraph.User)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(context.Context, libregraph.User) error); ok {
+		r1 = rf(ctx, user)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// DeleteGroup provides a mock function with given fields: ctx, id
+func (_m *Backend) DeleteGroup(ctx context.Context, id string) error {
+	ret := _m.Called(ctx, id)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(context.Context, string) error); ok {
+		r0 = rf(ctx, id)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+// DeleteUser provides a mock function with given fields: ctx, nameOrID
+func (_m *Backend) DeleteUser(ctx context.Context, nameOrID string) error {
+	ret := _m.Called(ctx, nameOrID)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(context.Context, string) error); ok {
+		r0 = rf(ctx, nameOrID)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+// GetGroup provides a mock function with given fields: ctx, nameOrID, queryParam
+func (_m *Backend) GetGroup(ctx context.Context, nameOrID string, queryParam url.Values) (*libregraph.Group, error) {
+	ret := _m.Called(ctx, nameOrID, queryParam)
+
+	var r0 *libregraph.Group
+	if rf, ok := ret.Get(0).(func(context.Context, string, url.Values) *libregraph.Group); ok {
+		r0 = rf(ctx, nameOrID, queryParam)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*libregraph.Group)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(context.Context, string, url.Values) error); ok {
+		r1 = rf(ctx, nameOrID, queryParam)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// GetGroupMembers provides a mock function with given fields: ctx, id
+func (_m *Backend) GetGroupMembers(ctx context.Context, id string) ([]*libregraph.User, error) {
+	ret := _m.Called(ctx, id)
+
+	var r0 []*libregraph.User
+	if rf, ok := ret.Get(0).(func(context.Context, string) []*libregraph.User); ok {
+		r0 = rf(ctx, id)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).([]*libregraph.User)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(context.Context, string) error); ok {
+		r1 = rf(ctx, id)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// GetGroups provides a mock function with given fields: ctx, queryParam
+func (_m *Backend) GetGroups(ctx context.Context, queryParam url.Values) ([]*libregraph.Group, error) {
+	ret := _m.Called(ctx, queryParam)
+
+	var r0 []*libregraph.Group
+	if rf, ok := ret.Get(0).(func(context.Context, url.Values) []*libregraph.Group); ok {
+		r0 = rf(ctx, queryParam)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).([]*libregraph.Group)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(context.Context, url.Values) error); ok {
+		r1 = rf(ctx, queryParam)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// GetUser provides a mock function with given fields: ctx, nameOrID, queryParam
+func (_m *Backend) GetUser(ctx context.Context, nameOrID string, queryParam url.Values) (*libregraph.User, error) {
+	ret := _m.Called(ctx, nameOrID, queryParam)
+
+	var r0 *libregraph.User
+	if rf, ok := ret.Get(0).(func(context.Context, string, url.Values) *libregraph.User); ok {
+		r0 = rf(ctx, nameOrID, queryParam)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*libregraph.User)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(context.Context, string, url.Values) error); ok {
+		r1 = rf(ctx, nameOrID, queryParam)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// GetUsers provides a mock function with given fields: ctx, queryParam
+func (_m *Backend) GetUsers(ctx context.Context, queryParam url.Values) ([]*libregraph.User, error) {
+	ret := _m.Called(ctx, queryParam)
+
+	var r0 []*libregraph.User
+	if rf, ok := ret.Get(0).(func(context.Context, url.Values) []*libregraph.User); ok {
+		r0 = rf(ctx, queryParam)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).([]*libregraph.User)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(context.Context, url.Values) error); ok {
+		r1 = rf(ctx, queryParam)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// RemoveMemberFromGroup provides a mock function with given fields: ctx, groupID, memberID
+func (_m *Backend) RemoveMemberFromGroup(ctx context.Context, groupID string, memberID string) error {
+	ret := _m.Called(ctx, groupID, memberID)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(context.Context, string, string) error); ok {
+		r0 = rf(ctx, groupID, memberID)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+// UpdateUser provides a mock function with given fields: ctx, nameOrID, user
+func (_m *Backend) UpdateUser(ctx context.Context, nameOrID string, user libregraph.User) (*libregraph.User, error) {
+	ret := _m.Called(ctx, nameOrID, user)
+
+	var r0 *libregraph.User
+	if rf, ok := ret.Get(0).(func(context.Context, string, libregraph.User) *libregraph.User); ok {
+		r0 = rf(ctx, nameOrID, user)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*libregraph.User)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(context.Context, string, libregraph.User) error); ok {
+		r1 = rf(ctx, nameOrID, user)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+type mockConstructorTestingTNewBackend interface {
+	mock.TestingT
+	Cleanup(func())
+}
+
+// NewBackend creates a new instance of Backend. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
+func NewBackend(t mockConstructorTestingTNewBackend) *Backend {
+	mock := &Backend{}
+	mock.Mock.Test(t)
+
+	t.Cleanup(func() { mock.AssertExpectations(t) })
+
+	return mock
+}
@@ -74,7 +74,7 @@ func (g Graph) GetRootDriveChildren(w http.ResponseWriter, r *http.Request) {
 	}

 	render.Status(r, http.StatusOK)
-	render.JSON(w, r, &listResponse{Value: files})
+	render.JSON(w, r, &ListResponse{Value: files})
 }

 func (g Graph) getDriveItem(ctx context.Context, root storageprovider.ResourceId) (*libregraph.DriveItem, error) {
@@ -96,7 +96,7 @@ func (g Graph) getDrives(w http.ResponseWriter, r *http.Request, unrestricted bo
 		if res.Status.Code == cs3rpc.Code_CODE_NOT_FOUND {
 			// return an empty list
 			render.Status(r, http.StatusOK)
-			render.JSON(w, r, &listResponse{})
+			render.JSON(w, r, &ListResponse{})
 			return
 		}
 		logger.Debug().Str("message", res.GetStatus().GetMessage()).Msg("could not get drives: grpc error")
@@ -126,7 +126,7 @@ func (g Graph) getDrives(w http.ResponseWriter, r *http.Request, unrestricted bo
 	}

 	render.Status(r, http.StatusOK)
-	render.JSON(w, r, &listResponse{Value: spaces})
+	render.JSON(w, r, &ListResponse{Value: spaces})
 }

 // GetSingleDrive does a lookup of a single space by spaceId
@@ -119,7 +119,7 @@ func (g Graph) getWebDavBaseURL() (*url.URL, error) {
 	return webDavBaseURL, nil
 }

-type listResponse struct {
+type ListResponse struct {
 	Value interface{} `json:"value,omitempty"`
 }

@@ -42,6 +42,7 @@ func (g Graph) GetGroups(w http.ResponseWriter, r *http.Request) {
 		} else {
 			errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error())
 		}
+		return
 	}

 	groups, err = sortGroups(odataReq, groups)
@@ -51,7 +52,7 @@ func (g Graph) GetGroups(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	render.Status(r, http.StatusOK)
-	render.JSON(w, r, &listResponse{Value: groups})
+	render.JSON(w, r, &ListResponse{Value: groups})
 }

 // PostGroup implements the Service interface.
@@ -312,20 +313,20 @@ func (g Graph) PostGroupMember(w http.ResponseWriter, r *http.Request) {
 	memberRefURL, ok := memberRef.GetOdataIdOk()
 	if !ok {
 		logger.Debug().Msg("could not add group member: @odata.id reference is missing")
-		errorcode.InvalidRequest.Render(w, r, http.StatusInternalServerError, "@odata.id reference is missing")
+		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "@odata.id reference is missing")
 		return
 	}
 	memberType, id, err := g.parseMemberRef(*memberRefURL)
 	if err != nil {
 		logger.Debug().Err(err).Msg("could not add group member: error parsing @odata.id url")
-		errorcode.InvalidRequest.Render(w, r, http.StatusInternalServerError, "Error parsing @odata.id url")
+		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "Error parsing @odata.id url")
 		return
 	}
 	// The MS Graph spec allows "directoryObject", "user", "group" and "organizational Contact"
 	// we restrict this to users for now. Might add Groups as members later
 	if memberType != "users" {
 		logger.Debug().Str("type", memberType).Msg("could not add group member: Only users are allowed as group members")
-		errorcode.InvalidRequest.Render(w, r, http.StatusInternalServerError, "Only users are allowed as group members")
+		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "Only users are allowed as group members")
 		return
 	}

@@ -371,14 +372,14 @@ func (g Graph) DeleteGroupMember(w http.ResponseWriter, r *http.Request) {
 	memberID := chi.URLParam(r, "memberID")
 	memberID, err = url.PathUnescape(memberID)
 	if err != nil {
-		logger.Debug().Err(err).Str("id", memberID).Msg("could not delete group member: unescaping group id failed")
-		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "unescaping group id failed")
+		logger.Debug().Err(err).Str("id", memberID).Msg("could not delete group member: unescaping member id failed")
+		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "unescaping member id failed")
 		return
 	}

 	if memberID == "" {
-		logger.Debug().Msg("could not delete group member: missing group id")
-		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "missing group id")
+		logger.Debug().Msg("could not delete group member: missing member id")
+		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "missing member id")
 		return
 	}
 	logger.Debug().Str("groupID", groupID).Str("memberID", memberID).Msg("calling delete member on backend")
@@ -0,0 +1,486 @@
+package svc_test
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"errors"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+
+	userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
+	ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
+	"github.com/go-chi/chi/v5"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"github.com/test-go/testify/mock"
+
+	libregraph "github.com/owncloud/libre-graph-api-go"
+	ogrpc "github.com/owncloud/ocis/v2/ocis-pkg/service/grpc"
+	"github.com/owncloud/ocis/v2/ocis-pkg/shared"
+	"github.com/owncloud/ocis/v2/services/graph/mocks"
+	"github.com/owncloud/ocis/v2/services/graph/pkg/config"
+	"github.com/owncloud/ocis/v2/services/graph/pkg/config/defaults"
+	identitymocks "github.com/owncloud/ocis/v2/services/graph/pkg/identity/mocks"
+	service "github.com/owncloud/ocis/v2/services/graph/pkg/service/v0"
+	"github.com/owncloud/ocis/v2/services/graph/pkg/service/v0/errorcode"
+)
+
+type groupList struct {
+	Value []*libregraph.Group
+}
+
+var _ = Describe("Groups", func() {
+	var (
+		svc             service.Service
+		ctx             context.Context
+		cfg             *config.Config
+		gatewayClient   *mocks.GatewayClient
+		eventsPublisher mocks.Publisher
+		identityBackend *identitymocks.Backend
+
+		rr *httptest.ResponseRecorder
+
+		newGroup    *libregraph.Group
+		currentUser = &userv1beta1.User{
+			Id: &userv1beta1.UserId{
+				OpaqueId: "user",
+			},
+		}
+	)
+
+	BeforeEach(func() {
+		eventsPublisher.On("Publish", mock.Anything, mock.Anything, mock.Anything).Return(nil)
+
+		identityBackend = &identitymocks.Backend{}
+		newGroup = libregraph.NewGroup()
+		newGroup.SetMembersodataBind([]string{"/users/user1"})
+		newGroup.SetId("group1")
+
+		rr = httptest.NewRecorder()
+		ctx = context.Background()
+
+		cfg = defaults.FullDefaultConfig()
+		cfg.Identity.LDAP.CACert = "" // skip the startup checks, we don't use LDAP at all in this tests
+		cfg.TokenManager.JWTSecret = "loremipsum"
+		cfg.Commons = &shared.Commons{}
+		cfg.GRPCClientTLS = &shared.GRPCClientTLS{}
+
+		_ = ogrpc.Configure(ogrpc.GetClientOptions(cfg.GRPCClientTLS)...)
+		svc = service.NewService(
+			service.Config(cfg),
+			service.WithGatewayClient(gatewayClient),
+			service.EventsPublisher(&eventsPublisher),
+			service.WithIdentityBackend(identityBackend),
+		)
+	})
+
+	Describe("GetGroups", func() {
+		It("handles invalid ODATA parameters", func() {
+			r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/groups?§foo=bar", nil)
+			svc.GetGroups(rr, r)
+
+			Expect(rr.Code).To(Equal(http.StatusBadRequest))
+		})
+
+		It("handles invalid sorting queries", func() {
+			identityBackend.On("GetGroups", ctx, mock.Anything).Return([]*libregraph.Group{newGroup}, nil)
+
+			r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/groups?$orderby=invalid", nil)
+			svc.GetGroups(rr, r)
+
+			Expect(rr.Code).To(Equal(http.StatusBadRequest))
+			data, err := ioutil.ReadAll(rr.Body)
+			Expect(err).ToNot(HaveOccurred())
+
+			odataerr := libregraph.OdataError{}
+			err = json.Unmarshal(data, &odataerr)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(odataerr.Error.Code).To(Equal("invalidRequest"))
+		})
+
+		It("handles unknown backend errors", func() {
+			identityBackend.On("GetGroups", ctx, mock.Anything).Return(nil, errors.New("failed"))
+
+			r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/groups", nil)
+			svc.GetGroups(rr, r)
+			Expect(rr.Code).To(Equal(http.StatusInternalServerError))
+			data, err := ioutil.ReadAll(rr.Body)
+			Expect(err).ToNot(HaveOccurred())
+
+			odataerr := libregraph.OdataError{}
+			err = json.Unmarshal(data, &odataerr)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(odataerr.Error.Code).To(Equal("generalException"))
+		})
+
+		It("handles backend errors", func() {
+			identityBackend.On("GetGroups", ctx, mock.Anything).Return(nil, errorcode.New(errorcode.AccessDenied, "access denied"))
+
+			r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/groups", nil)
+			svc.GetGroups(rr, r)
+
+			Expect(rr.Code).To(Equal(http.StatusInternalServerError))
+			data, err := ioutil.ReadAll(rr.Body)
+			Expect(err).ToNot(HaveOccurred())
+
+			odataerr := libregraph.OdataError{}
+			err = json.Unmarshal(data, &odataerr)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(odataerr.Error.Code).To(Equal("accessDenied"))
+		})
+
+		It("renders an empty list of groups", func() {
+			identityBackend.On("GetGroups", ctx, mock.Anything).Return([]*libregraph.Group{}, nil)
+
+			r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/groups", nil)
+			svc.GetGroups(rr, r)
+
+			Expect(rr.Code).To(Equal(http.StatusOK))
+			data, err := ioutil.ReadAll(rr.Body)
+			Expect(err).ToNot(HaveOccurred())
+
+			res := service.ListResponse{}
+			err = json.Unmarshal(data, &res)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(res.Value).To(Equal([]interface{}{}))
+		})
+
+		It("renders a list of groups", func() {
+			identityBackend.On("GetGroups", ctx, mock.Anything).Return([]*libregraph.Group{newGroup}, nil)
+
+			r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/groups", nil)
+			svc.GetGroups(rr, r)
+
+			Expect(rr.Code).To(Equal(http.StatusOK))
+			data, err := ioutil.ReadAll(rr.Body)
+			Expect(err).ToNot(HaveOccurred())
+
+			res := groupList{}
+			err = json.Unmarshal(data, &res)
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(len(res.Value)).To(Equal(1))
+			Expect(res.Value[0].GetId()).To(Equal("group1"))
+		})
+	})
+
+	Describe("GetGroup", func() {
+		It("handles missing or empty group id", func() {
+			r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/groups", nil)
+			svc.GetGroup(rr, r)
+
+			Expect(rr.Code).To(Equal(http.StatusBadRequest))
+
+			r = httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/groups", nil)
+			rctx := chi.NewRouteContext()
+			rctx.URLParams.Add("groupID", "")
+			r = r.WithContext(context.WithValue(ctxpkg.ContextSetUser(ctx, nil), chi.RouteCtxKey, rctx))
+			svc.GetGroup(rr, r)
+
+			Expect(rr.Code).To(Equal(http.StatusBadRequest))
+		})
+
+		Context("with an existing group", func() {
+			BeforeEach(func() {
+				identityBackend.On("GetGroup", mock.Anything, mock.Anything, mock.Anything).Return(newGroup, nil)
+			})
+
+			It("gets the group", func() {
+				r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/groups/"+*newGroup.Id, nil)
+				rctx := chi.NewRouteContext()
+				rctx.URLParams.Add("groupID", *newGroup.Id)
+				r = r.WithContext(context.WithValue(ctxpkg.ContextSetUser(ctx, nil), chi.RouteCtxKey, rctx))
+
+				svc.GetGroup(rr, r)
+
+				Expect(rr.Code).To(Equal(http.StatusOK))
+			})
+		})
+	})
+
+	Describe("PostGroup", func() {
+		It("handles invalid body", func() {
+			r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/me/groups/", bytes.NewBufferString("{invalid"))
+
+			svc.PostGroup(rr, r)
+			Expect(rr.Code).To(Equal(http.StatusBadRequest))
+		})
+
+		It("handles missing display name", func() {
+			newGroup = libregraph.NewGroup()
+			newGroup.SetId("disallowed")
+			newGroup.SetMembersodataBind([]string{"/non-users/user"})
+			newGroupJson, err := json.Marshal(newGroup)
+			Expect(err).ToNot(HaveOccurred())
+
+			r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/me/groups/", bytes.NewBuffer(newGroupJson))
+
+			svc.PostGroup(rr, r)
+			Expect(rr.Code).To(Equal(http.StatusBadRequest))
+		})
+
+		It("disallows user create ids", func() {
+			newGroup = libregraph.NewGroup()
+			newGroup.SetId("disallowed")
+			newGroup.SetDisplayName("New Group")
+			newGroup.SetMembersodataBind([]string{"/non-users/user"})
+			newGroupJson, err := json.Marshal(newGroup)
+			Expect(err).ToNot(HaveOccurred())
+
+			r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/me/groups/", bytes.NewBuffer(newGroupJson))
+
+			svc.PostGroup(rr, r)
+			Expect(rr.Code).To(Equal(http.StatusBadRequest))
+		})
+
+		It("creates the group", func() {
+			newGroup = libregraph.NewGroup()
+			newGroup.SetDisplayName("New Group")
+			newGroupJson, err := json.Marshal(newGroup)
+			Expect(err).ToNot(HaveOccurred())
+
+			identityBackend.On("CreateGroup", mock.Anything, mock.Anything).Return(newGroup, nil)
+
+			r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/me/groups/", bytes.NewBuffer(newGroupJson))
+
+			svc.PostGroup(rr, r)
+
+			Expect(rr.Code).To(Equal(http.StatusOK))
+		})
+	})
+	Describe("PatchGroup", func() {
+		It("handles invalid body", func() {
+			r := httptest.NewRequest(http.MethodPatch, "/graph/v1.0/me/groups/", bytes.NewBufferString("{invalid"))
+			rctx := chi.NewRouteContext()
+			rctx.URLParams.Add("groupID", *newGroup.Id)
+			r = r.WithContext(context.WithValue(ctxpkg.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
+			svc.PatchGroup(rr, r)
+			Expect(rr.Code).To(Equal(http.StatusBadRequest))
+		})
+
+		It("handles missing or empty group id", func() {
+			r := httptest.NewRequest(http.MethodPatch, "/graph/v1.0/me/groups", nil)
+			svc.PatchGroup(rr, r)
+
+			Expect(rr.Code).To(Equal(http.StatusBadRequest))
+
+			r = httptest.NewRequest(http.MethodPatch, "/graph/v1.0/me/groups", nil)
+			rctx := chi.NewRouteContext()
+			rctx.URLParams.Add("groupID", "")
+			r = r.WithContext(context.WithValue(ctxpkg.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
+			svc.PatchGroup(rr, r)
+
+			Expect(rr.Code).To(Equal(http.StatusBadRequest))
+		})
+
+		Context("with an existing group", func() {
+			BeforeEach(func() {
+				identityBackend.On("GetGroup", mock.Anything, mock.Anything, mock.Anything).Return(newGroup, nil)
+			})
+
+			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"})
+				updatedGroupJson, err := json.Marshal(updatedGroup)
+				Expect(err).ToNot(HaveOccurred())
+
+				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(ctxpkg.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
+				svc.PatchGroup(rr, r)
+
+				Expect(rr.Code).To(Equal(http.StatusBadRequest))
+			})
+
+			It("fails on invalid user refs", func() {
+				updatedGroup := libregraph.NewGroup()
+				updatedGroup.SetDisplayName("group1 updated")
+				updatedGroup.SetMembersodataBind([]string{"invalid"})
+				updatedGroupJson, err := json.Marshal(updatedGroup)
+				Expect(err).ToNot(HaveOccurred())
+
+				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(ctxpkg.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
+				svc.PatchGroup(rr, r)
+
+				Expect(rr.Code).To(Equal(http.StatusBadRequest))
+			})
+
+			It("fails when the adding non-users users", func() {
+				updatedGroup := libregraph.NewGroup()
+				updatedGroup.SetDisplayName("group1 updated")
+				updatedGroup.SetMembersodataBind([]string{"/non-users/user1"})
+				updatedGroupJson, err := json.Marshal(updatedGroup)
+				Expect(err).ToNot(HaveOccurred())
+
+				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(ctxpkg.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
+				svc.PatchGroup(rr, r)
+
+				Expect(rr.Code).To(Equal(http.StatusBadRequest))
+			})
+
+			It("adds members to the group", func() {
+				identityBackend.On("AddMembersToGroup", mock.Anything, mock.Anything, mock.Anything).Return(nil)
+
+				updatedGroup := libregraph.NewGroup()
+				updatedGroup.SetDisplayName("group1 updated")
+				updatedGroup.SetMembersodataBind([]string{"/users/user1"})
+				updatedGroupJson, err := json.Marshal(updatedGroup)
+				Expect(err).ToNot(HaveOccurred())
+
+				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(ctxpkg.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
+				svc.PatchGroup(rr, r)
+
+				Expect(rr.Code).To(Equal(http.StatusNoContent))
+				identityBackend.AssertNumberOfCalls(GinkgoT(), "AddMembersToGroup", 1)
+			})
+		})
+	})
+
+	Describe("DeleteGroup", func() {
+		Context("with an existing group", func() {
+			BeforeEach(func() {
+				identityBackend.On("GetGroup", mock.Anything, mock.Anything, mock.Anything).Return(newGroup, nil)
+			})
+		})
+
+		It("deletes the group", func() {
+			identityBackend.On("DeleteGroup", mock.Anything, mock.Anything, mock.Anything).Return(nil)
+			r := httptest.NewRequest(http.MethodPatch, "/graph/v1.0/me/groups", nil)
+			rctx := chi.NewRouteContext()
+			rctx.URLParams.Add("groupID", *newGroup.Id)
+			r = r.WithContext(context.WithValue(ctxpkg.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
+			svc.DeleteGroup(rr, r)
+
+			Expect(rr.Code).To(Equal(http.StatusNoContent))
+			identityBackend.AssertNumberOfCalls(GinkgoT(), "DeleteGroup", 1)
+			eventsPublisher.AssertNumberOfCalls(GinkgoT(), "Publish", 1)
+		})
+	})
+
+	Describe("GetGroupMembers", func() {
+		It("gets the list of members", func() {
+			user := libregraph.NewUser()
+			user.SetId("user")
+			identityBackend.On("GetGroupMembers", mock.Anything, mock.Anything, mock.Anything).Return([]*libregraph.User{user}, nil)
+
+			r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/groups/{groupID}/members", nil)
+			rctx := chi.NewRouteContext()
+			rctx.URLParams.Add("groupID", *newGroup.Id)
+			r = r.WithContext(context.WithValue(ctxpkg.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
+			svc.GetGroupMembers(rr, r)
+			Expect(rr.Code).To(Equal(http.StatusOK))
+
+			data, err := ioutil.ReadAll(rr.Body)
+			Expect(err).ToNot(HaveOccurred())
+
+			var members []*libregraph.User
+			err = json.Unmarshal(data, &members)
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(len(members)).To(Equal(1))
+			Expect(members[0].GetId()).To(Equal("user"))
+		})
+	})
+
+	Describe("PostGroupMembers", func() {
+		It("fails on invalid body", func() {
+			r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/me/groups/{groupID}/members", bytes.NewBufferString("{invalid"))
+			rctx := chi.NewRouteContext()
+			rctx.URLParams.Add("groupID", *newGroup.Id)
+			r = r.WithContext(context.WithValue(ctxpkg.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
+			svc.PostGroupMember(rr, r)
+			Expect(rr.Code).To(Equal(http.StatusBadRequest))
+		})
+
+		It("fails on missing member refs", func() {
+			member := libregraph.NewMemberReference()
+			data, err := json.Marshal(member)
+			Expect(err).ToNot(HaveOccurred())
+
+			r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/me/groups/{groupID}/members", bytes.NewBuffer(data))
+			rctx := chi.NewRouteContext()
+			rctx.URLParams.Add("groupID", *newGroup.Id)
+			r = r.WithContext(context.WithValue(ctxpkg.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
+			svc.PostGroupMember(rr, r)
+			Expect(rr.Code).To(Equal(http.StatusBadRequest))
+		})
+
+		It("fails on invalid member refs", func() {
+			member := libregraph.NewMemberReference()
+			member.SetOdataId("/invalidtype/user")
+			data, err := json.Marshal(member)
+			Expect(err).ToNot(HaveOccurred())
+
+			r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/me/groups/{groupID}/members", bytes.NewBuffer(data))
+			rctx := chi.NewRouteContext()
+			rctx.URLParams.Add("groupID", *newGroup.Id)
+			r = r.WithContext(context.WithValue(ctxpkg.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
+			svc.PostGroupMember(rr, r)
+			Expect(rr.Code).To(Equal(http.StatusBadRequest))
+		})
+
+		It("adds a new member", func() {
+			member := libregraph.NewMemberReference()
+			member.SetOdataId("/users/user")
+			data, err := json.Marshal(member)
+			Expect(err).ToNot(HaveOccurred())
+			identityBackend.On("AddMembersToGroup", mock.Anything, mock.Anything, mock.Anything).Return(nil)
+
+			r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/me/groups/{groupID}/members", bytes.NewBuffer(data))
+			rctx := chi.NewRouteContext()
+			rctx.URLParams.Add("groupID", *newGroup.Id)
+			r = r.WithContext(context.WithValue(ctxpkg.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
+			svc.PostGroupMember(rr, r)
+			Expect(rr.Code).To(Equal(http.StatusNoContent))
+
+			identityBackend.AssertNumberOfCalls(GinkgoT(), "AddMembersToGroup", 1)
+		})
+	})
+
+	Describe("DeleteGroupMembers", func() {
+		It("handles missing or empty member id", func() {
+			r := httptest.NewRequest(http.MethodDelete, "/graph/v1.0/me/groups/{groupID}/members/{memberID}/$ref", nil)
+			rctx := chi.NewRouteContext()
+			rctx.URLParams.Add("groupID", *newGroup.Id)
+			r = r.WithContext(context.WithValue(ctxpkg.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
+			svc.DeleteGroupMember(rr, r)
+			Expect(rr.Code).To(Equal(http.StatusBadRequest))
+		})
+		It("handles missing or empty member id", func() {
+			r := httptest.NewRequest(http.MethodDelete, "/graph/v1.0/me/groups/{groupID}/members/{memberID}/$ref", nil)
+			rctx := chi.NewRouteContext()
+			rctx.URLParams.Add("memberID", "/users/user")
+			r = r.WithContext(context.WithValue(ctxpkg.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
+			svc.DeleteGroupMember(rr, r)
+			Expect(rr.Code).To(Equal(http.StatusBadRequest))
+		})
+
+		It("deletes members", func() {
+			identityBackend.On("RemoveMemberFromGroup", mock.Anything, mock.Anything, mock.Anything).Return(nil)
+
+			r := httptest.NewRequest(http.MethodDelete, "/graph/v1.0/me/groups/{groupID}/members/{memberID}/$ref", nil)
+			rctx := chi.NewRouteContext()
+			rctx.URLParams.Add("groupID", *newGroup.Id)
+			rctx.URLParams.Add("memberID", "/users/user1")
+			r = r.WithContext(context.WithValue(ctxpkg.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
+			svc.DeleteGroupMember(rr, r)
+			Expect(rr.Code).To(Equal(http.StatusNoContent))
+
+			identityBackend.AssertNumberOfCalls(GinkgoT(), "RemoveMemberFromGroup", 1)
+		})
+	})
+})
@@ -95,7 +95,7 @@ func (g Graph) GetUsers(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	render.Status(r, http.StatusOK)
-	render.JSON(w, r, &listResponse{Value: users})
+	render.JSON(w, r, &ListResponse{Value: users})
 }

 func (g Graph) PostUser(w http.ResponseWriter, r *http.Request) {