Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

graph: user groupTypes property to indicate read-only groups #5974

Merged
merged 2 commits into from
Apr 4, 2023

Conversation

rhafer
Copy link
Contributor

@rhafer rhafer commented Mar 30, 2023

By setting GRAPH_LDAP_GROUP_CREATE_BASE_DN a distinct subtree can be configure
where new LDAP groups are created. That subtree needs to be subordinate to
GRAPH_LDAP_GROUP_BASE_DN. All groups outside for
GRAPH_LDAP_GROUP_CREATE_BASE_DN are considered read-only and only group below
that DN can be updated and deleted.

This is introduced for a pretty specific usecase where most groups are managed
in an external source (e.g. a read-only replica of an LDAP tree). But we still
want to allow the local administrator to create groups in a writeable subtree
attached to that replica.

To test this you can start ocis with

IDM_CREATE_DEMO_USERS=true GRAPH_LDAP_GROUP_CREATE_BASE_DN=ou=users,o=libregraph-idm GRAPH_LDAP_GROUP_BASE_DN=o=libregraph-idm ocis/bin/ocis server

This will make all the demo groups "readonly" (indicated through "ReadOnly" value in "groupTypes":

~> curl -s -k -u admin:admin  'https://localhost:9200/graph/v1.0/groups/users'   -H 'accept: application/json' | jq .
{
  "displayName": "users",
  "groupTypes": [
    "ReadOnly"
  ],
  "id": "509a9dcd-bb37-4f4f-a01a-19dca27d9cfa"
}

Trying to update the group will end with an error (currently a 500), but I still need to fix that to return a 405 or 403.

Adding groups is still possible, but they'll endup in a seperate part of the LDAP tree:

~> curl -s -k -u admin:admin  'https://localhost:9200/graph/v1.0/groups'   -H 'accept: application/json' -X POST --data '{"displayName": "Lokal"}' | jq .
{
  "displayName": "Lokal",
  "groupTypes": [],
  "id": "0a0f201c-c38f-40fd-a4c4-cd87d9168ee9"
}

Groups without the "ReadOnly" value in the groupTypes can be updated:

~> curl -i -k -u admin:admin  'https://localhost:9200/graph/v1.0/groups/0a0f201c-c38f-40fd-a4c4-cd87d9168ee9'   -H 'accept: application/json' -X PATCH --data '{"displayName": "Lokale Gruppe"}' 
HTTP/1.1 204 No Content
Date: Thu, 30 Mar 2023 16:48:47 GMT
X-Graph-Version: 3.0.0-alpha.1+093d65dcd

@rhafer rhafer self-assigned this Mar 30, 2023
@update-docs
Copy link

update-docs bot commented Mar 30, 2023

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@rhafer
Copy link
Contributor Author

rhafer commented Mar 30, 2023

@JammingBen This should be of interest for you.

Note: We opted for using the groupTypes list property for indicating the readOnly state of a group as we didn't want to introduce some incompatible (with MS graph) property on the groups resource. If that is too clumsy for you I am open for discussion. But I guess @micbar and @butonic have some opinion about this as well.

@ownclouders
Copy link
Contributor

ownclouders commented Mar 30, 2023

@JammingBen
Copy link
Contributor

@JammingBen This should be of interest for you.

Note: We opted for using the groupTypes list property for indicating the readOnly state of a group as we didn't want to introduce some incompatible (with MS graph) property on the groups resource. If that is too clumsy for you I am open for discussion. But I guess @micbar and @butonic have some opinion about this as well.

That's okay, I can work with that. Thanks for the heads-up!

@rhafer rhafer marked this pull request as ready for review April 3, 2023 13:53
@rhafer rhafer requested a review from kobergj April 3, 2023 13:53
By setting GRAPH_LDAP_GROUP_CREATE_BASE_DN a distinct subtree can be
configured where new LDAP groups are created. That subtree needs to be
subordinate to GRAPH_LDAP_GROUP_BASE_DN. All groups outside for
GRAPH_LDAP_GROUP_CREATE_BASE_DN are considered read-only and only groups
below that DN can be updated and deleted.

This is introduced for a pretty specific usecase where most groups are managed
in an external source (e.g. a read-only replica of an LDAP tree). But we still
want to allow the local administrator to create groups in a writeable subtree
attached to that replica.
@sonarcloud
Copy link

sonarcloud bot commented Apr 4, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

25.9% 25.9% Coverage
0.0% 0.0% Duplication

@rhafer rhafer merged commit 120887a into owncloud:master Apr 4, 2023
@micbar micbar mentioned this pull request May 3, 2023
89 tasks
@ScharfViktor ScharfViktor mentioned this pull request May 4, 2023
86 tasks
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.

4 participants