Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions docs/developer/admin-utils.rst
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,39 @@ This class has two important attributes:
<https://github.com/openwisp/openwisp-firmware-upgrader/search?q=multitenant_parent>`_
for a real-world example.

Disabled Organization Write Protection
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

``MultitenantAdminMixin`` also blocks changes to any object belonging to a
:ref:`disabled organization <disabling_an_organization>`, while still
allowing that object to be viewed and deleted. This applies to superusers
too: there is no per-user bypass (the only way to opt out is the
class-level attribute described below). For models whose organization is
reached through a parent (via ``multitenant_parent``), the mixin traverses
the parent to find the organization, so those child objects are protected
as well.

This is controlled by the ``disabled_organization_write_protection`` class
attribute, which defaults to ``True``. Set it to ``False`` on a specific
``ModelAdmin`` to opt out:

.. code-block:: python

from django.contrib import admin
from openwisp_users.multitenancy import MultitenantAdminMixin


class BookAdmin(MultitenantAdminMixin, admin.ModelAdmin):
disabled_organization_write_protection = False
# other attributes

The ``organization`` form field's queryset also excludes disabled
organizations for everyone, superusers included, so a disabled
organization can never be *selected* for a new object. The one exception
is an object that already belongs to a disabled organization on a
``ModelAdmin`` with the opt-out set: its own (disabled) organization stays
selectable in the field so the existing value can still be saved.

``MultitenantOrgFilter``
------------------------

Expand Down
67 changes: 66 additions & 1 deletion docs/developer/django-rest-framework-utils.rst
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,69 @@ organization managers or owners to view shared objects in read-only mode.

Standard users will not be able to view or list shared objects.

``DisabledOrgReadOnly``
~~~~~~~~~~~~~~~~~~~~~~~

**Full python path**:
``openwisp_users.api.permissions.DisabledOrgReadOnly``.

This object-level permission class blocks updating an object that belongs
to a :ref:`disabled organization <disabling_an_organization>`. Read (safe
methods) and ``DELETE`` remain allowed.

.. important::

``DisabledOrgReadOnly`` guards **updates only**. It implements
``has_object_permission``, which DRF does not call on ``POST``, so it
does **not** block *creating* a new object for a disabled
organization. Create protection instead relies on the organization
field excluding disabled organizations: use one of the
``FilterSerializerByOrganization`` mixins (or a related field backed
by ``Organization.active``) on the serializer. A plain
``ModelSerializer`` whose organization field defaults to
``Organization.objects`` will happily create objects for a disabled
organization even under ``ProtectedAPIMixin``.

A view can opt out of this guard by setting
``allow_disabled_organization_writes = True``:

.. code-block:: python

from openwisp_users.api.permissions import DisabledOrgReadOnly
from rest_framework.generics import RetrieveUpdateDestroyAPIView


class SubnetView(RetrieveUpdateDestroyAPIView):
permission_classes = (DisabledOrgReadOnly,)
allow_disabled_organization_writes = True
# other attributes

``DisabledOrgReadOnly`` is already included in ``ProtectedAPIMixin``'s
default ``permission_classes`` (see below), so views that use
``ProtectedAPIMixin`` get this guard automatically without any extra
configuration.

.. note::

``Organization.active`` (django-organizations' ``ActiveOrgManager``)
is the canonical queryset for active organizations: use
``Organization.active.all()`` when writing custom code that needs to
select from or filter active organizations, instead of filtering
``Organization.objects`` manually.

``ProtectedAPIMixin``
---------------------

**Full python path**: ``openwisp_users.api.mixins.ProtectedAPIMixin``.

This mixin provides a set of authentication and permission classes that
are commonly used across various OpenWISP modules API views.
are commonly used across various OpenWISP modules API views, including
``DisabledOrgReadOnly`` (see above).

If a view overrides ``permission_classes`` entirely instead of extending
``ProtectedAPIMixin.permission_classes``, it will not inherit
``DisabledOrgReadOnly`` (or any future addition to the mixin's defaults)
automatically, and must re-declare it explicitly if the guard is needed.

Usage example:

Expand Down Expand Up @@ -255,6 +311,15 @@ and ``FilterSerializerByOrgOwned`` can be used to solve this issue.

These serializers do not allow non-superusers to create shared objects.

.. _multi_tenant_serializers_disabled_org:

The ``organization`` field's queryset also excludes :ref:`disabled
organizations <disabling_an_organization>`, for everyone, superusers
included, so a disabled organization can never be selected when creating
or updating an object. Submitting the primary key of a disabled
organization returns a validation error explaining that the organization
does not exist or is disabled.

Usage example:

.. code-block:: python
Expand Down
50 changes: 50 additions & 0 deletions docs/user/basic-concepts.rst
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,56 @@ instance of the platform.
`django-organizations
<https://github.com/bennylope/django-organizations>`_ third-party app.

.. _disabling_an_organization:

Disabling an Organization
-------------------------

Superusers and managers of the organization can disable it, by unchecking
its **Is active** flag on the "Change organization" page or via the REST
API (subject to the usual permission requirements for editing an
organization).

Disabling an organization does not delete anything: all of its data,
including users, memberships, and related objects, remains fully
**readable** and **deletable**. What changes is:

- **No new object can be created for a disabled organization**, and
**existing objects belonging to it cannot be modified**, superusers
included. This applies to the organization's own record too: once
disabled, only its **Is active** flag can be changed (to re-enable it)
or its owner unassigned; everything else is locked until it is
re-enabled.
- Deleting objects, including the organization itself, is always allowed,
so cleanup is never blocked.
- The organization stops appearing in **organization selection widgets**
(e.g. when creating a new object), so it can no longer be picked for new
data. It still appears in admin **list filters**, so its existing data
remains easy to find for auditing purposes.
- Re-enabling a disabled organization is allowed **only for superusers**.
Once an organization is disabled, its managers lose access to it (a
disabled organization is no longer part of the organizations they
manage), so they can no longer edit it, including re-enabling it. A
superuser must re-enable the organization before its managers regain
access.
Comment on lines +162 to +183

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Clarify who can still read and delete disabled-organization data.

“Fully readable and deletable” conflicts with the later statement that managers lose access. State that data remains available to users who retain permission, notably superusers.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/user/basic-concepts.rst` around lines 162 - 183, Update the opening
statement in the disabled-organization documentation to clarify that its data
remains readable and deletable only by users who retain the relevant
permissions, notably superusers. Keep the existing restrictions on managers and
other users consistent with the later access description.


.. note::

In the REST API, attempting to update an object belonging to a
disabled organization returns an HTTP 400 or 403 response with a clear
error message, instead of failing silently or being blocked without
explanation.

.. note::

Re-enabling an organization and editing its other fields must be done
in **two separate steps**, matching the admin interface (which locks
every field except **Is active** while the organization is disabled).
First re-enable the organization (change only **Is active**), then
edit its other fields or assign an owner. A single request that both
re-enables the organization and changes another field (or assigns an
owner) is rejected.

Organization Membership and Roles
---------------------------------

Expand Down
106 changes: 98 additions & 8 deletions openwisp_users/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
from . import settings as app_settings
from .multitenancy import MultitenantAdminMixin, MultitenantOrgFilter
from .utils import BaseAdmin
from .widgets import OrganizationAutocompleteSelect

Group = load_model("openwisp_users", "Group")
Organization = load_model("openwisp_users", "Organization")
Expand Down Expand Up @@ -98,34 +99,100 @@ class OrganizationOwnerInline(admin.StackedInline):
extra = 0
autocomplete_fields = ("organization_user",)

def has_add_permission(self, request, obj=None):
# obj is the parent Organization here
if obj is not None and not obj.is_active:
return False
return super().has_add_permission(request, obj)

def has_change_permission(self, request, obj=None):
if obj is not None and not obj.is_active:
return False
if obj and not request.user.is_superuser and not request.user.is_owner(obj):
return False
return super().has_change_permission(request, obj)


class OrganizationUserInlineFormSet(RequiredInlineFormSet):
"""
Renders existing memberships of a disabled organization as read-only so
the row survives a no-op save (the disabled organization is not part of
the field queryset otherwise) and its select widget shows the disabled
organization instead of rendering empty. Deleting the row stays possible.
"""

def add_fields(self, form, index):
super().add_fields(form, index)
instance = getattr(form, "instance", None)
if (
instance
and instance.pk
and instance.organization_id
and not instance.organization.is_active
):
org_field = form.fields.get("organization")
if org_field is not None:
org_field.disabled = True
org_field.queryset = Organization.objects.filter(
pk=instance.organization_id
)
if "is_admin" in form.fields:
form.fields["is_admin"].disabled = True
Comment thread
coderabbitai[bot] marked this conversation as resolved.


class OrganizationUserInline(admin.StackedInline):
model = OrganizationUser
formset = RequiredInlineFormSet
formset = OrganizationUserInlineFormSet
view_on_site = False
fields = ("organization", "is_admin")
autocomplete_fields = ("organization",)

def get_queryset(self, request):
# OrganizationUserInlineFormSet.add_fields() reads
# instance.organization.is_active for every row; select_related
# folds that per-row query into this one.
return super().get_queryset(request).select_related("organization")

def get_formset(self, request, obj=None, **kwargs):
"""
In form dropdowns, display only organizations
in which operator `is_admin` and for superusers
display all organizations
In form dropdowns, display only active organizations;
non-superusers additionally only see organizations
in which they are `is_admin`.
"""
formset = super().get_formset(request, obj=obj, **kwargs)
org_field = formset.form.base_fields["organization"]
org_field.queryset = org_field.queryset.filter(is_active=True)
if request.user.is_superuser:
return formset
if not request.user.is_superuser:
formset.form.base_fields["organization"].queryset = (
Organization.objects.filter(pk__in=request.user.organizations_managed)
)
org_field.queryset = org_field.queryset.filter(
pk__in=request.user.organizations_managed
)
return formset

def formfield_for_foreignkey(self, db_field, request, **kwargs):
"""
Route the organization picker through the ``ow-auto-filter`` endpoint
so disabled organizations are excluded from the dropdown for everyone,
superusers included (the stock ``admin:autocomplete`` endpoint does not
filter them). Only replaces the widget when the field is actually an
autocomplete field, so that disabling ``autocomplete_fields`` keeps
rendering a plain select.
"""
if db_field.name == "organization" and db_field.name in (
self.get_autocomplete_fields(request)
):
kwargs["widget"] = OrganizationAutocompleteSelect(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In manual testing, the user add page freezes the browser on this branch. The new organization autocomplete is now used by this inline, but org-autocomplete.js assumes there is a select#id_organization on the page. Inline fields use ids like id_openwisp_users_organizationuser-0-organization, so the jQuery selection is empty and the parent-walking loop never reaches a form. Please check the autocomplete initialization for inline usage. A minimal fix could be to skip the script when the expected field is missing, or make it target the actual widget instance instead of a hardcoded id.

db_field, self.admin_site, using=kwargs.get("using")
)
return super().formfield_for_foreignkey(db_field, request, **kwargs)

def has_add_permission(self, request, obj=None):
# an operator who manages no active organization cannot pick one, so
# the add row would be unusable: hide it
if not request.user.is_superuser and not request.user.organizations_managed:
return False
return super().has_add_permission(request, obj)

def get_extra(self, request, obj=None, **kwargs):
if not obj:
return 1
Expand Down Expand Up @@ -583,6 +650,29 @@ def has_change_permission(self, request, obj=None):
return False
return super().has_change_permission(request, obj)

def get_readonly_fields(self, request, obj=None):
"""
A disabled organization can only be re-enabled: every other
field becomes readonly (owner unassignment is still possible
via the inline's delete action, which does not go through here).
"""
fields = super().get_readonly_fields(request, obj)
if obj and not obj.is_active:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In manual testing, disabling an organization makes the main organization fields read-only, but downstream inlines such as config management settings, organization variables, and geo settings remain writable. Please look into whether this can be handled centrally for organization admin inlines, or through the multitenancy admin utilities, instead of requiring each downstream inline to implement the same guard. Inline writes for disabled organizations need test coverage because they are another path for modifying disabled-organization data.

editable_fields = [
f.name
for f in self.model._meta.local_fields
if f.editable and f.name != "is_active"
]
fields = list(fields) + [f for f in editable_fields if f not in fields]
return fields

def get_prepopulated_fields(self, request, obj=None):
# prepopulated_fields cannot reference a field that is also
# readonly, which is the case for "slug" on a disabled organization
if obj and not obj.is_active:
return {}
return super().get_prepopulated_fields(request, obj)

class Media(CopyableFieldsAdmin.Media):
css = {"all": ("openwisp-users/css/admin.css",)}

Expand Down
Loading
Loading