Skip to content

Add endpoints for managing group membership#23

Merged
rhafer merged 1 commit intoowncloud:mainfrom
rhafer:groups
Jan 27, 2022
Merged

Add endpoints for managing group membership#23
rhafer merged 1 commit intoowncloud:mainfrom
rhafer:groups

Conversation

@rhafer
Copy link
Copy Markdown
Contributor

@rhafer rhafer commented Jan 20, 2022

  • POST /groups/{id}/members/$ref (for adding a single member)
  • DELETE graph/v1.0/groups/{id}/members/{id}/$ref (for deleting a single member)
  • PATCH graph/v1.0/groups/{group-id} (for adding multiple members)

@rhafer rhafer marked this pull request as ready for review January 25, 2022 15:53
@rhafer
Copy link
Copy Markdown
Contributor Author

rhafer commented Jan 25, 2022

  • PATCH graph/v1.0/groups/{group-id} (for adding multiple members)

I am having some troube with adding the definition for being able to add multiple group members at once (using the above URL). According to the MS Docs multiple members can be added with PATCH to the above URL and a body containing something like this:

{
  "members@odata.bind": [
    "https://graph.microsoft.com/v1.0/directoryObjects/{id}",
    "https://graph.microsoft.com/v1.0/directoryObjects/{id}",
    "https://graph.microsoft.com/v1.0/directoryObjects/{id}"
    ]
}

We however already have PATCH operation defined for that URL using a different kind of body (basically for updating other group related attributes.

@micbar Any hints on how to solve this? (BTW, this shouldn't stop of from merging this PR. We can work it out later)

@micbar
Copy link
Copy Markdown
Contributor

micbar commented Jan 25, 2022

shouldn't we be able to create a key "members@odata.bind" which has a collection of directory objects as a value?

@rhafer
Copy link
Copy Markdown
Contributor Author

rhafer commented Jan 26, 2022

shouldn't we be able to create a key "members@odata.bind"

@micbar hm, true. That should work. For some reason I thought the body for the PATCH on /groups/{id} required some additional attributes.Please review once more.

I still think the API for managing group memberships as defined in ms graph is not exactly nice. I am trying to follow it anyway here.

which has a collection of directory objects as a value?

Not really directory objects, but strings/uris referencing directory objects.

Comment thread api/openapi-spec/v0.0.yaml Outdated
Comment thread api/openapi-spec/v0.0.yaml Outdated
@refs
Copy link
Copy Markdown
Member

refs commented Jan 27, 2022

shouldn't we be able to create a key "members@odata.bind" which has a collection of directory objects as a value?

@rhafer something like: https://swagger.io/docs/specification/data-models/data-types/#array

Also, mind this restriction by the msgraph spec:

Note that up to 20 members can be added in a single request

The key would, obviously, be located in the PATCH request body

@rhafer
Copy link
Copy Markdown
Contributor Author

rhafer commented Jan 27, 2022

shouldn't we be able to create a key "members@odata.bind" which has a collection of directory objects as a value?

@rhafer something like: https://swagger.io/docs/specification/data-models/data-types/#array

Yeah that's what I did. :-)

Also, mind this restriction by the msgraph spec:

Note that up to 20 members can be added in a single request

The key would, obviously, be located in the PATCH request body

Yeah. Though that limit is not something I can express in this SPEC, is it? We might think about adding such a limit to the actual graph API implementation at some point though.

@refs
Copy link
Copy Markdown
Member

refs commented Jan 27, 2022

yes it can be done within the spec: https://swagger.io/docs/specification/2-0/describing-parameters/

Additionally, you can:

use minItems and maxItems to control the size of the array,
enforce uniqueItems,
- in: query
  name: color
  required: false
  type: array
  minItems: 1
  maxItems: 5
  uniqueItems: true
  items:
    type: string
    enum: [black, white, gray, red, pink, orange, yellow, green, blue, purple, brown]

@refs
Copy link
Copy Markdown
Member

refs commented Jan 27, 2022

@rhafer oh sorry, those docs are for v2. For v3: https://swagger.io/docs/specification/data-models/data-types/

image

- add a group member via
  POST /groups/{id}/members/$ref
- delete a group member via
  DELETE /groups/{group-id}/members/{directory-object-id}/$ref
- to allow adding multiple members add the "members@odata.bind" property
  to groups
@rhafer
Copy link
Copy Markdown
Contributor Author

rhafer commented Jan 27, 2022

Ok, thanks. I've added those limits to the spec. It doesn't seem to have any effect on the generated go code for the client. But the limit itself needs to be implemented server side anyway.

@rhafer rhafer merged commit f521b24 into owncloud:main Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants