openwisp / openwisp/openwisp-users
[bug] Resending an unchanged is_admin value deletes the OrganizationUser membership
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 179
- Forks
- 96
- Avg merge
- 13h 40m
- Merged PRs (30d)
- 16
Description
Describe the bug
SuperUserDetailSerializer.update() in openwisp_users/api/serializers.py uses a non-obvious rule for organization_users: if the payload includes an existing membership and the submitted is_admin value is the same as the current value, the membership is deleted instead of being left unchanged.
This behavior is implemented here:
if org_user.is_admin != org_user_data["is_admin"]:
org_user.is_admin = org_user_data["is_admin"]
org_user.full_clean()
org_user.save()
else:
org_user.delete()
As a result, API clients that resend the current representation of a user on PATCH can unintentionally delete organization memberships. The only way to preserve an existing membership is to omit it entirely from the payload, or omit is_admin from that specific entry.
Why this is a problem
- The delete-vs-update behavior is not documented in the REST API docs.
- The serializer response representation includes
organization_users, so a naive PATCH round-trip can be destructive. - This is easy to miss for API consumers who expect unchanged fields to remain unchanged.
Relevant code
openwisp_users/api/serializers.pyBaseSuperUserSerializer.to_representation()serializesorganization_userswithorganizationandis_admin.SuperUserDetailSerializer.update()deletes the membership whenis_adminmatches the current value.
docs/user/rest-api.rstcurrently notes thatis_adminrepresents the organization manager flag, but does not document the delete-on-unchanged behavior.- Existing tests such as
test_remove_org_user_apiandtest_make_user_org_admin_apicover parts of this flow, but do not make the contract explicit in the docs.
Proposed resolution
Please make a product decision on one of these options:
- Document the current behavior explicitly, or
- Replace it with an explicit removal signal instead of overloading "unchanged" to mean "delete".
Option 2 would be a breaking API change for existing consumers, so it likely needs a migration plan or versioning strategy.
Notes
This is unrelated to the disabled-organization write protection work. The behavior predates that effort and appears to come from the original REST API implementation.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with SuperUserDetailSerializer.update() and BaseSuperUserSerializer.to_representation() in openwisp_users/api/serializers.py, then read docs/user/rest-api.rst and the tests test_remove_org_user_api and test_make_user_org_admin_api. The issue requires a product decision between documenting the current behavior and changing the removal signal; done means the chosen contract is explicit and covered by the relevant documentation and tests, with migration or versioning considered for a breaking change.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100