add/remove multiple users from group via one request

[AlexAndBear]

Describe the solution you'd like

To keep the backend request for web limited, we need the possibility to add/remove multiple users from a group via one request

MS Graph also provides an example for this
https://learn.microsoft.com/en-us/graph/api/group-post-members?view=graph-rest-1.0&tabs=http

PATCH https://graph.microsoft.com/v1.0/groups/{group-id}
Content-type: application/json

{
  "[email protected]": [
    "https://graph.microsoft.com/v1.0/directoryObjects/{id}",
    "https://graph.microsoft.com/v1.0/directoryObjects/{id}",
    "https://graph.microsoft.com/v1.0/directoryObjects/{id}"
    ]
}

[AlexAndBear]

FYI @rhafer @butonic

[rhafer]

@janackermann For adding multiple members this is already implemented since quite a while.

For removing multiple members MS Graph doesn't make any specification IIRC. Also I am unsure if this is really worth it. Yeah, you'd save a couple of roundtrips between web and graph, but OTOH the user management APIs are not that performance critical. It's not that we constantly add and remove groupmembers in batches.

[rhafer]

@janackermann BTW, please note that there is a limit of how many members you can add in one batch. (AFAIK it defaults to 20)

[AlexAndBear]

@rhafer
Thank you.
I think it is crucial that we add those information (batch limit) to the specs e.G. swagger.

Having a consistency for adding/removing multiple users from groups would be nice anyways.

[rhafer]

I think it is crucial that we add those information (batch limit) to the specs e.G. swagger.

It actually is in the spec https://github.com/owncloud/libre-graph-api/blob/main/api/openapi-spec/v1.0.yaml#L2669
for what ever reason it just doesn't show up in the swagger UI.

[AlexAndBear]

@rhafer oh that's weird tho, I always have a look in the swagger UI or here https://owncloud.dev/apis/http/graph/groups instead of some yaml files.
Anyways, I would love to implement the chunking for add/remove users from groups if both endpoints exist and we have a consistency

[rhafer]

Anyways, I would love to implement the chunking for add/remove users from groups if both endpoints exist and we have a consistency

Yeah, I get that. However there currently does not seem to be any endpoint for deleting multiple members in MS Graph. I also have no idea how a deletion of multiple members is correctly represented in an ODATA PATCH request currently.
The odata 4.01 specs talk about Delta Payloads in PATCH request for representing these kind of changes, but this is something that neither MS Graph nor any of our tooling seems to currently support. We could of course decide to diverge from MS here, but I'd challenge the idea that this part of the provisioning API (adding and removing multiple group members) is critical enough to justify spending quite some engineering effort on saving a couple of API round trips.

[micbar]

Just asking, what happens when a single group fails in the request?

We have the API promise that we don't do "bad things" like doing non-atomic operations which could fail silently.

[rhafer]

Just asking, what happens when a single group fails in the request?

We currently only allow adding multiple users to a single group in one request. In the LDAP backend that boils down to a single LDAP request on the group. That request either succeeds completely (all members added) or fails (no member added)

We have the API promise that we don't do "bad things" like doing non-atomic operations which could fail silently.

I think at least for the "add multiple user to a group at once" operation this promise is ful-filled.

Somehow I think adding support for https://learn.microsoft.com/en-us/graph/api/group-post-members?view=graph-rest-1.0&tabs=http#example-2-add-multiple-members-to-a-group-in-a-single-request to libregraph was a bad idea.

Especially as it lacks the counter part of being able to remove users from a group in batch. And (as you might have noticed 😄 ) I am really hesitant to invent something libregraph specific here.

[ScharfViktor]

I guess it's not going to be fixed. closed as not planned
please re-open if I'm wrong