Add support for adding multiple members to a group at once

Adding multiple members at once is done via PATCH on
`groups/{group-oid}` with a body containing a list of refernces to the
members.

graph/pkg/identity/backend.go

@@ -11,13 +11,10 @@ import (
 type Backend interface {
 	// CreateUser creates a given user in the identity backend.
 	CreateUser(ctx context.Context, user libregraph.User) (*libregraph.User, error)
-
 	// DeleteUser deletes a given user, identified by username or id, from the backend
 	DeleteUser(ctx context.Context, nameOrID string) error
-
 	// UpdateUser applies changes to given user, identified by username or id
 	UpdateUser(ctx context.Context, nameOrID string, user libregraph.User) (*libregraph.User, error)
-
 	GetUser(ctx context.Context, nameOrID string) (*libregraph.User, error)
 	GetUsers(ctx context.Context, queryParam url.Values) ([]*libregraph.User, error)
 
@@ -28,8 +25,8 @@ type Backend interface {
 	GetGroup(ctx context.Context, nameOrID string) (*libregraph.Group, error)
 	GetGroups(ctx context.Context, queryParam url.Values) ([]*libregraph.Group, error)
 	GetGroupMembers(ctx context.Context, id string) ([]*libregraph.User, error)
-	// AddMemberToGroup adds a new member (reference by ID) to supplied group in the identity backend.
+	// AddMembersToGroup adds new members (reference by a slice of IDs) to supplied group in the identity backend.
-	AddMemberToGroup(ctx context.Context, groupID string, memberID string) error
+	AddMembersToGroup(ctx context.Context, groupID string, memberID []string) error
 	// RemoveMemberFromGroup removes a single member (by ID) from a group
 	RemoveMemberFromGroup(ctx context.Context, groupID string, memberID string) error
 }

graph/pkg/identity/cs3.go

@@ -184,8 +184,8 @@ func (i *CS3) GetGroupMembers(ctx context.Context, groupID string) ([]*libregrap
 	return nil, errorcode.New(errorcode.NotSupported, "not implemented")
 }
 
-// AddMemberToGroup implements the Backend Interface. It's currently not supported for the CS3 backend
+// AddMembersToGroup implements the Backend Interface. It's currently not supported for the CS3 backend
-func (i *CS3) AddMemberToGroup(ctx context.Context, groupID string, memberID string) error {
+func (i *CS3) AddMembersToGroup(ctx context.Context, groupID string, memberID []string) error {
 	return errorcode.New(errorcode.NotSupported, "not implemented")
 }
 

graph/pkg/identity/ldap.go

@@ -568,19 +568,14 @@ func (i *LDAP) DeleteGroup(ctx context.Context, id string) error {
 	return nil
 }
 
-// AddMemberToGroup implements the Backend Interface for the LDAP backend.
+// AddMembersToGroup implements the Backend Interface for the LDAP backend.
 // Currently it is limited to adding Users as Group members. Adding other groups
 // as members is not yet implemented
-func (i *LDAP) AddMemberToGroup(ctx context.Context, groupID string, memberID string) error {
+func (i *LDAP) AddMembersToGroup(ctx context.Context, groupID string, memberIDs []string) error {
 	ge, err := i.getLDAPGroupByID(groupID, true)
 	if err != nil {
 		return err
 	}
-	me, err := i.getLDAPUserByID(memberID)
-	if err != nil {
-		return err
-	}
-	i.logger.Debug().Str("backend", "ldap").Str("groupdn", ge.DN).Str("member", me.DN).Msg("Add Member")
 
 	mr := ldap.ModifyRequest{DN: ge.DN}
 	// Handle empty groups (using the empty member attribute)
@@ -588,29 +583,47 @@ func (i *LDAP) AddMemberToGroup(ctx context.Context, groupID string, memberID st
 	if len(current) == 1 && current[0] == "" {
 		mr.Delete(i.groupAttributeMap.member, []string{""})
 	}
-	nUserDN, err := ldapdn.ParseNormalize(me.DN)
-	for _, member := range current {
-		if member == "" {
-			continue
-		}
-		if nMember, err := ldapdn.ParseNormalize(member); err != nil {
-			// We couldn't parse the member value as a DN. Let's keep it
-			// as it is but log a warning
-			i.logger.Warn().Str("memberDN", member).Err(err).Msg("Couldn't parse DN")
-			continue
-		} else {
-			if nMember == nUserDN {
-				i.logger.Info().Str("memberDN", member).Msg("User already present. Nothing to do")
-				return nil
-			}
-		}
 
+	// Create a Set of current members for faster lookups
+	currentSet := make(map[string]struct{}, len(current))
+	for _, currentMember := range current {
+		// We can ignore any empty member value here
+		if currentMember == "" {
+			continue
+		}
+		nCurrentMember, err := ldapdn.ParseNormalize(currentMember)
+		if err != nil {
+			// We couldn't parse the member value as a DN. Let's skip it, but log a warning
+			i.logger.Warn().Str("memberDN", currentMember).Err(err).Msg("Couldn't parse DN")
+			continue
+		}
+		currentSet[nCurrentMember] = struct{}{}
 	}
 
-	mr.Add(i.groupAttributeMap.member, []string{me.DN})
+	var newMemberDNs []string
+	for _, memberID := range memberIDs {
+		me, err := i.getLDAPUserByID(memberID)
+		if err != nil {
+			return err
+		}
+		nDN, err := ldapdn.ParseNormalize(me.DN)
+		if err != nil {
+			i.logger.Error().Str("new member", me.DN).Err(err).Msg("Couldn't parse DN")
+			return err
+		}
+		if _, present := currentSet[nDN]; !present {
+			newMemberDNs = append(newMemberDNs, me.DN)
+		} else {
+			i.logger.Debug().Str("memberDN", me.DN).Msg("Member already present in group. Skipping")
+		}
+	}
 
-	if err := i.conn.Modify(&mr); err != nil {
+	if len(newMemberDNs) > 0 {
-		return err
+		mr.Add(i.groupAttributeMap.member, newMemberDNs)
+
+		if err := i.conn.Modify(&mr); err != nil {
+			return err
+		}
 	}
 	return nil
 }
@@ -630,6 +643,10 @@ func (i *LDAP) RemoveMemberFromGroup(ctx context.Context, groupID string, member
 	i.logger.Debug().Str("backend", "ldap").Str("groupdn", ge.DN).Str("member", me.DN).Msg("remove member")
 
 	nOldMemberDN, err := ldapdn.ParseNormalize(me.DN)
+	if err != nil {
+		i.logger.Error().Str("old member", me.DN).Err(err).Msg("Couldn't parse DN")
+		return err
+	}
 	members := ge.GetEqualFoldAttributeValues(i.groupAttributeMap.member)
 	found := false
 	for _, member := range members {

graph/pkg/service/v0/groups.go

@@ -3,6 +3,7 @@ package svc
 import (
 	"encoding/json"
 	"errors"
+	"fmt"
 	"net/http"
 	"net/url"
 	"strings"
@@ -14,6 +15,8 @@ import (
 	"github.com/go-chi/render"
 )
 
+const memberRefsLimit = 20
+
 // GetGroups implements the Service interface.
 func (g Graph) GetGroups(w http.ResponseWriter, r *http.Request) {
 	groups, err := g.identityBackend.GetGroups(r.Context(), r.URL.Query())
@@ -61,6 +64,66 @@ func (g Graph) PostGroup(w http.ResponseWriter, r *http.Request) {
 	render.JSON(w, r, grp)
 }
 
+// PatchGroup implements the Service interface.
+func (g Graph) PatchGroup(w http.ResponseWriter, r *http.Request) {
+	g.logger.Debug().Msg("Calling PatchGroup")
+	groupID := chi.URLParam(r, "groupID")
+	groupID, err := url.PathUnescape(groupID)
+	if err != nil {
+		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "unescaping group id failed")
+		return
+	}
+
+	if groupID == "" {
+		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "missing group id")
+		return
+	}
+	changes := libregraph.NewGroup()
+	err = json.NewDecoder(r.Body).Decode(changes)
+	if err != nil {
+		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error())
+		return
+	}
+
+	if memberRefs, ok := changes.GetMembersodataBindOk(); ok {
+		// The spec defines a limit of 20 members maxium per Request
+		if len(memberRefs) > memberRefsLimit {
+			errorcode.NotAllowed.Render(w, r, http.StatusInternalServerError,
+				fmt.Sprintf("Request is limited to %d members", memberRefsLimit))
+			return
+		}
+		memberIDs := make([]string, 0, len(memberRefs))
+		for _, memberRef := range memberRefs {
+			memberType, id, err := g.parseMemberRef(memberRef)
+			if err != nil {
+				errorcode.InvalidRequest.Render(w, r, http.StatusInternalServerError, "Error parsing member@odata.bind values")
+				return
+			}
+			g.logger.Debug().Str("memberType", memberType).Str("memberid", id).Msg("Add Member")
+			// 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" {
+				errorcode.InvalidRequest.Render(w, r, http.StatusInternalServerError, "Only user are allowed as group members")
+				return
+			}
+			memberIDs = append(memberIDs, id)
+		}
+		err = g.identityBackend.AddMembersToGroup(r.Context(), groupID, memberIDs)
+	}
+
+	if err != nil {
+		var errcode errorcode.Error
+		if errors.As(err, &errcode) {
+			errcode.Render(w, r)
+		} else {
+			errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error())
+		}
+		return
+	}
+	render.Status(r, http.StatusNoContent)
+	render.NoContent(w, r)
+}
+
 // GetGroup implements the Service interface.
 func (g Graph) GetGroup(w http.ResponseWriter, r *http.Request) {
 	groupID := chi.URLParam(r, "groupID")
@@ -171,18 +234,11 @@ func (g Graph) PostGroupMember(w http.ResponseWriter, r *http.Request) {
 		errorcode.InvalidRequest.Render(w, r, http.StatusInternalServerError, "@odata.id refernce is missing")
 		return
 	}
-	memberURL, err := url.ParseRequestURI(*memberRefURL)
+	memberType, id, err := g.parseMemberRef(*memberRefURL)
 	if err != nil {
 		errorcode.InvalidRequest.Render(w, r, http.StatusInternalServerError, "Error parsing @odata.id url")
 		return
 	}
-	segments := strings.Split(memberURL.Path, "/")
-	if len(segments) < 2 {
-		errorcode.InvalidRequest.Render(w, r, http.StatusInternalServerError, "Error parsing @odata.id url path")
-		return
-	}
-	id := segments[len(segments)-1]
-	memberType := segments[len(segments)-2]
 	// 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" {
@@ -191,7 +247,7 @@ func (g Graph) PostGroupMember(w http.ResponseWriter, r *http.Request) {
 	}
 
 	g.logger.Debug().Str("memberType", memberType).Str("id", id).Msg("Add Member")
-	err = g.identityBackend.AddMemberToGroup(r.Context(), groupID, id)
+	err = g.identityBackend.AddMembersToGroup(r.Context(), groupID, []string{id})
 
 	if err != nil {
 		var errcode errorcode.Error
@@ -248,3 +304,17 @@ func (g Graph) DeleteGroupMember(w http.ResponseWriter, r *http.Request) {
 	render.Status(r, http.StatusNoContent)
 	render.NoContent(w, r)
 }
+
+func (g Graph) parseMemberRef(ref string) (string, string, error) {
+	memberURL, err := url.ParseRequestURI(ref)
+	if err != nil {
+		return "", "", err
+	}
+	segments := strings.Split(memberURL.Path, "/")
+	if len(segments) < 2 {
+		return "", "", errors.New("invalid member reference")
+	}
+	id := segments[len(segments)-1]
+	memberType := segments[len(segments)-2]
+	return memberType, id, nil
+}

graph/pkg/service/v0/instrument.go

@@ -69,6 +69,11 @@ func (i instrument) PostGroup(w http.ResponseWriter, r *http.Request) {
 	i.next.PostGroup(w, r)
 }
 
+// PatchGroup implements the Service interface.
+func (i instrument) PatchGroup(w http.ResponseWriter, r *http.Request) {
+	i.next.PatchGroup(w, r)
+}
+
 // DeleteGroup implements the Service interface.
 func (i instrument) DeleteGroup(w http.ResponseWriter, r *http.Request) {
 	i.next.DeleteGroup(w, r)

graph/pkg/service/v0/logging.go

@@ -69,6 +69,11 @@ func (l logging) PostGroup(w http.ResponseWriter, r *http.Request) {
 	l.next.PostGroup(w, r)
 }
 
+// PatchGroup implements the Service interface.
+func (l logging) PatchGroup(w http.ResponseWriter, r *http.Request) {
+	l.next.PatchGroup(w, r)
+}
+
 // DeleteGroup implements the Service interface.
 func (l logging) DeleteGroup(w http.ResponseWriter, r *http.Request) {
 	l.next.DeleteGroup(w, r)

graph/pkg/service/v0/service.go

@@ -34,6 +34,7 @@ type Service interface {
 	GetGroups(http.ResponseWriter, *http.Request)
 	GetGroup(http.ResponseWriter, *http.Request)
 	PostGroup(http.ResponseWriter, *http.Request)
+	PatchGroup(http.ResponseWriter, *http.Request)
 	DeleteGroup(http.ResponseWriter, *http.Request)
 	GetGroupMembers(http.ResponseWriter, *http.Request)
 	PostGroupMember(http.ResponseWriter, *http.Request)
@@ -121,6 +122,7 @@ func NewService(opts ...Option) Service {
 				r.Route("/{groupID}", func(r chi.Router) {
 					r.Get("/", svc.GetGroup)
 					r.Delete("/", svc.DeleteGroup)
+					r.Patch("/", svc.PatchGroup)
 					r.Route("/members", func(r chi.Router) {
 						r.Get("/", svc.GetGroupMembers)
 						r.Post("/$ref", svc.PostGroupMember)

graph/pkg/service/v0/tracing.go

@@ -65,6 +65,11 @@ func (t tracing) PostGroup(w http.ResponseWriter, r *http.Request) {
 	t.next.PostGroup(w, r)
 }
 
+// PatchGroup implements the Service interface.
+func (t tracing) PatchGroup(w http.ResponseWriter, r *http.Request) {
+	t.next.PatchGroup(w, r)
+}
+
 // DeleteGroup implements the Service interface.
 func (t tracing) DeleteGroup(w http.ResponseWriter, r *http.Request) {
 	t.next.DeleteGroup(w, r)