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-ldap: Fix possible races when editing group membership in parallel #6214

Merged
merged 1 commit into from
May 3, 2023

Conversation

rhafer
Copy link
Contributor

@rhafer rhafer commented May 3, 2023

As the standard LDAP groups (groupOfNames) require at least one "member" value to be present in a group, we have workarounds in place that add an empty member ("") when creating a new group or when removing the last member from the group. This can cause a race condition when e.g. multiple request to remove members from a group an running in parallel, as we need to read the group before we can construct the modification request. If some other request modified the group (e.g. deleted the 2nd last member) after we read it, we create non-working modification request.

These changes try to catch those errors and retry the modification request once.

Fixes: #6170

As the standard LDAP groups (groupOfNames) require at least one "member"
value to be present in a group, we have workarounds in place that add an
empty member ("") when creating a new group or when removing the last
member from the group. This can cause a race condition when e.g. multiple
request to remove members from a group an running in parallel, as we need
to read the group before we can construct the modification request. If
some other request modified the group (e.g. deleted the 2nd last member)
after we read it, we create non-working modification request.

These changes try to catch those errors and retry the modification
request once.

Fixes: owncloud#6170
@rhafer rhafer requested a review from micbar May 3, 2023 11:34
@rhafer rhafer self-assigned this May 3, 2023
@update-docs
Copy link

update-docs bot commented May 3, 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.

@sonarqubecloud
Copy link

sonarqubecloud bot commented May 3, 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 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@butonic butonic merged commit f1dbe43 into owncloud:master May 3, 2023
ownclouders pushed a commit that referenced this pull request May 3, 2023
…lel (#6214)

As the standard LDAP groups (groupOfNames) require at least one "member"
value to be present in a group, we have workarounds in place that add an
empty member ("") when creating a new group or when removing the last
member from the group. This can cause a race condition when e.g. multiple
request to remove members from a group an running in parallel, as we need
to read the group before we can construct the modification request. If
some other request modified the group (e.g. deleted the 2nd last member)
after we read it, we create non-working modification request.

These changes try to catch those errors and retry the modification
request once.

Fixes: #6170
fschade pushed a commit that referenced this pull request Jul 10, 2023
…lel (#6214)

As the standard LDAP groups (groupOfNames) require at least one "member"
value to be present in a group, we have workarounds in place that add an
empty member ("") when creating a new group or when removing the last
member from the group. This can cause a race condition when e.g. multiple
request to remove members from a group an running in parallel, as we need
to read the group before we can construct the modification request. If
some other request modified the group (e.g. deleted the 2nd last member)
after we read it, we create non-working modification request.

These changes try to catch those errors and retry the modification
request once.

Fixes: #6170
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.

Ocis vs ldap. 500 error when adding multiple users to a group
3 participants