[bug] Resending an unchanged is_admin value deletes the OrganizationUser membership

Aperta
  • Qualcosa non va?
#552 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
5/5
Tempo stimato
Più di una settimana
Idoneità per principianti
35/100
Tipo di issue
Bug
Chiarezza
Da chiarire
Stato di attività
Tranquilla
Stack tecnologico
python
Ambito
api, backend

Direzione di ricerca

Inizia da SuperUserDetailSerializer.update() e BaseSuperUserSerializer.to_representation() in openwisp_users/api/serializers.py, poi leggi docs/user/rest-api.rst e i test test_remove_org_user_api e test_make_user_org_admin_api. L'issue richiede una decisione di prodotto tra documentare il comportamento attuale e modificare il segnale di rimozione; il lavoro è completato quando il contratto scelto è esplicito e coperto dalla documentazione e dai test pertinenti, considerando la migrazione o il versionamento in caso di modifica incompatibile.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

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.py
    • BaseSuperUserSerializer.to_representation() serializes organization_users with organization and is_admin.
    • SuperUserDetailSerializer.update() deletes the membership when is_admin matches the current value.
  • docs/user/rest-api.rst currently notes that is_admin represents the organization manager flag, but does not document the delete-on-unchanged behavior.
  • Existing tests such as test_remove_org_user_api and test_make_user_org_admin_api cover 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:

  1. Document the current behavior explicitly, or
  2. 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.

Lingua principale
Python
Stelle
180
Fork
98
Merge medio
18h 13m
PR unite (30g)
13

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di openwisp/openwisp-users

Tutte le issue di openwisp/openwisp-users

Issue simili

Altre issue su Python

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.