[feature] Made RegisteredUser model support multi-tenancy #692

.github/workflows/ci.yml

@@ -74,6 +74,7 @@ jobs:
           pip install -U pip wheel setuptools
           pip install -U -r requirements-test.txt
           pip install -e .[saml,openvpn_status]
+          pip install --upgrade --no-deps --no-cache-dir --force-reinstall "https://github.com/openwisp/openwisp-users/tarball/issues/497-export-users"
           pip install ${{ matrix.django-version }}
 
       - name: Start InfluxDB and Redis container

docs/user/rest-api.rst

@@ -803,6 +803,48 @@ Param        Description
 phone_number string
 ============ ===========
 
+Update Registered User Method
++++++++++++++++++++++++++++++
+
+**Requires the user auth token (Bearer Token)**.
+
+Allows users to update their registered user method for an organization.
+The method can only be updated when it is currently set to
+``pending_verification``. Once updated, it cannot be changed again via
+this endpoint.
+
+This endpoint is used during cross-organization login when a user
+authenticates to a new organization. The user must complete verification
+for that organization before they can create account with the new
+organization.
+
+.. code-block:: text
+
+    /api/v1/radius/organization/<organization-slug>/account/registration-method/
+
+Responds only to **POST**.
+
+Parameters:
+
+====== ===========
+Param  Description
+====== ===========
+method string (\*)
+====== ===========
+
+(\*) ``method`` must be one of the available
+:ref:`registration/verification methods
+<openwisp_radius_needs_identity_verification>`, excluding
+``pending_verification``.
+
+**Success Response (200 OK)**:
+
+.. code-block:: json
+
+    {
+        "method": "mobile_phone"
+    }
+
 .. _radius_batch_user_creation:
 
 Batch user creation

docs/user/settings.rst

@@ -696,6 +696,9 @@ verification method. The following choices are available by default:
 - ``mobile_phone``: Mobile phone number :ref:`verification via SMS
   <openwisp_radius_sms_verification_enabled>`
 - ``social_login``: :doc:`social login feature <social_login>`
+- ``pending_verification``: Transitional state used when a user
+  authenticates to a new organization but has not yet completed
+  verification for that organization.
 
 .. note::
 

openwisp_radius/admin.py

@@ -539,6 +539,7 @@ class RegisteredUserInline(StackedInline):
     form = AlwaysHasChangedForm
     extra = 0
     readonly_fields = ("modified",)
+    fields = ("organization", "method", "is_verified", "modified")
 
     def has_delete_permission(self, request, obj=None):
         return False
@@ -549,12 +550,17 @@ def has_delete_permission(self, request, obj=None):
     RadiusUserGroupInline,
     PhoneTokenInline,
 ]
-UserAdmin.list_filter += (RegisteredUserFilter, "registered_user__method")
+UserAdmin.list_filter += (RegisteredUserFilter, "registered_users__method")
 
 
 def get_is_verified(self, obj):
     try:
-        value = "yes" if obj.registered_user.is_verified else "no"
+        if not obj.registered_users.exists():
+            value = "unknown"
+        elif obj.registered_users.filter(is_verified=True).exists():
+            value = "yes"
+        else:
+            value = "no"
     except Exception:
         value = "unknown"
     icon_url = static(f"admin/img/icon-{value}.svg")
@@ -564,7 +570,6 @@ def get_is_verified(self, obj):
 UserAdmin.get_is_verified = get_is_verified
 UserAdmin.get_is_verified.short_description = _("Verified")
 UserAdmin.list_display.insert(3, "get_is_verified")
-UserAdmin.list_select_related = ("registered_user",)
 
 
 class OrganizationRadiusSettingsInline(admin.StackedInline):
@@ -622,7 +627,7 @@ class Media:
 # avoid cluttering the admin with too many models, leave only the
 # minimum required to configure social login and check if it's working
 if app_settings.SOCIAL_REGISTRATION_CONFIGURED:
-    from allauth.socialaccount.admin import SocialAccount, SocialApp, SocialAppAdmin
+    from allauth.socialaccount.admin import SocialAccount
 
     class SocialAccountInline(admin.StackedInline):
         model = SocialAccount
@@ -636,7 +641,6 @@ def has_delete_permission(self, request, obj=None):
             return False
 
     UserAdmin.inlines += [SocialAccountInline]
-    admin.site.register(SocialApp, SocialAppAdmin)
 
 
 if app_settings.USER_ADMIN_RADIUSTOKEN_INLINE:

openwisp_radius/api/freeradius_views.py

@@ -7,7 +7,7 @@
 from django.contrib.auth.models import AnonymousUser
 from django.core.cache import cache
 from django.db import IntegrityError
-from django.db.models import Q
+from django.db.models import Exists, OuterRef, Q
 from django.utils.translation import gettext_lazy as _
 from django_filters import rest_framework as filters
 from django_filters.rest_framework import DjangoFilterBackend
@@ -57,6 +57,7 @@
 
 RadiusToken = load_model("RadiusToken")
 RadiusAccounting = load_model("RadiusAccounting")
+RegisteredUser = load_model("RegisteredUser")
 OrganizationRadiusSettings = load_model("OrganizationRadiusSettings")
 OrganizationUser = swapper.load_model("openwisp_users", "OrganizationUser")
 Organization = swapper.load_model("openwisp_users", "Organization")
@@ -290,7 +291,7 @@ def get_user(self, request, username, password):
         """
         conditions = self._get_user_query_conditions(request)
         try:
-            user = auth_backend.get_users(username).filter(conditions)[0]
+            user = auth_backend.get_users(username).filter(conditions).distinct()[0]
         except IndexError:
             return None
         # ensure user is member of the authenticated org
@@ -405,23 +406,45 @@ def _check_counters(self, data, user, group, group_checks):
     def _get_user_query_conditions(self, request):
         is_active = Q(is_active=True)
         needs_verification = self._needs_identity_verification({"pk": request._auth})
-        # if no identity verification enabled for this org,
-        # just ensure user is active
         if not needs_verification:
             return is_active
-        # if identity verification is enabled
-        is_verified = Q(registered_user__is_verified=True)
+        organization_id = request._auth
         AUTHORIZE_UNVERIFIED = registration.AUTHORIZE_UNVERIFIED
-        # and no method should authorize unverified users
-        # ensure user is active AND verified
+        # Use subqueries to ensure org-specific records take precedence over
+        # global (organization=NULL) records.
+        # A JOIN-based filter would allow a user to pass if ANY registered_users
+        # row matched, causing a bypass when a global verified record coexisted
+        # with an org-specific unverified record.
+        #
+        # Strategy: check if org-specific record exists and satisfies criteria;
+        # if not, fall back to checking the global record. This matches the
+        # behavior in api/utils.py:IDVerificationHelper.is_identity_verified_strong.
+        org_specific = RegisteredUser.objects.filter(
+            user=OuterRef("pk"),
+            organization_id=organization_id,
+        )
+        global_only = RegisteredUser.objects.filter(
+            user=OuterRef("pk"),
+            organization_id__isnull=True,
+        )
+
+        # is_verified: user passes if org-specific record is verified, or if
+        # no org-specific record exists and the global record is verified.
+        has_org_verified = Exists(org_specific.filter(is_verified=True))
+        has_global_verified = Exists(global_only.filter(is_verified=True))
+        no_org_specific = ~Exists(org_specific.values("pk"))
+        is_verified = has_org_verified | (no_org_specific & has_global_verified)
+
         if not AUTHORIZE_UNVERIFIED:
             return is_active & is_verified
-        # in case some methods are allowed to authorize unverified users
-        # ensure user is active AND
-        # (user is verified OR user uses one of these methods)
-        else:
-            authorize_unverified = Q(registered_user__method__in=AUTHORIZE_UNVERIFIED)
-            return is_active & (is_verified | authorize_unverified)
+
+        # authorize_unverified: user passes if org-specific record uses a
+        # special method, or if no org-specific record exists and the global
+        # record uses a special method.
+        has_org_special = Exists(org_specific.filter(method__in=AUTHORIZE_UNVERIFIED))
+        has_global_special = Exists(global_only.filter(method__in=AUTHORIZE_UNVERIFIED))
+        authorize_unverified = has_org_special | (no_org_specific & has_global_special)
+        return is_active & (is_verified | authorize_unverified)
 
     def authenticate_user(self, request, user, password):
         """

openwisp_radius/api/serializers.py

@@ -36,7 +36,7 @@
 from .. import settings as app_settings
 from ..base.forms import PasswordResetForm
 from ..counters.exceptions import SkipCheck
-from ..registration import REGISTRATION_METHOD_CHOICES
+from ..registration import get_registration_choices
 from ..utils import (
     get_group_checks,
     get_organization_radius_settings,
@@ -571,7 +571,7 @@ class RegisterSerializer(
             'verification in its "Organization RADIUS Settings."'
         ),
         default="",
-        choices=REGISTRATION_METHOD_CHOICES,
+        choices=get_registration_choices(),
     )
 
     def validate_phone_number(self, phone_number):
@@ -688,9 +688,11 @@ def save(self, request):
         # the custom_signup method contains the openwisp specific logic
         self.custom_signup(request, user)
         # create a RegisteredUser object for every user that registers through API
-        RegisteredUser.objects.create(
+        org = self.context["view"].organization
+        RegisteredUser.objects.get_or_create(
             user=user,
-            method=self.validated_data["method"],
+            organization=org,
+            defaults={"method": self.validated_data["method"]},
         )
         setup_user_email(request, user, [])
         return user
@@ -753,20 +755,64 @@ def save(self):
         # yet, tha will be done by the phone token validation view
         # once the phone number has been validated
         # at this point we flag the user as unverified again
-        self.user.registered_user.is_verified = False
-        self.user.registered_user.save()
+        org = self.context["view"].organization
+        reg_user, _ = RegisteredUser.get_or_create_for_user_and_org(
+            user=self.user,
+            organization=org,
+            defaults={"is_verified": False, "method": ""},
+        )
+        reg_user.is_verified = False
+        reg_user.save()
+
+
+class UpdateRegisteredUserMethodSerializer(ValidatedModelSerializer):
+    method = serializers.ChoiceField(
+        choices=get_registration_choices(),
+        help_text=_(
+            "The registration method to set for the user. "
+            "Cannot be 'pending_verification'."
+        ),
+    )
+
+    class Meta:
+        model = RegisteredUser
+        fields = ["method"]
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.fields["method"].choices = get_registration_choices()
+
+    def validate_method(self, value):
+        if value == "pending_verification":
+            raise serializers.ValidationError(
+                _("'pending_verification' cannot be set as a registration method.")
+            )
+        return value
+
+    def validate(self, attrs):
+        if self.instance.method != "pending_verification":
+            raise serializers.ValidationError(
+                {
+                    "method": _(
+                        "Method can only be updated from pending verification state."
+                    )
+                }
+            )
+        return attrs
+
+    def update(self, instance, validated_data):
+        instance.method = validated_data["method"]
+        instance.save()
+        return instance
 
 
 class RadiusUserSerializer(serializers.ModelSerializer):
     """
     Used to return information about the logged in user
     """
 
-    is_verified = serializers.BooleanField(source="registered_user.is_verified")
-    method = serializers.CharField(
-        source="registered_user.method",
-        allow_null=True,
-    )
+    is_verified = serializers.SerializerMethodField()
+    method = serializers.SerializerMethodField()
     password_expired = serializers.BooleanField(source="has_password_expired")
     radius_user_token = serializers.CharField(source="radius_token.key", default=None)
 
@@ -786,3 +832,33 @@ class Meta:
             "password_expired",
             "radius_user_token",
         ]
+
+    def _get_registered_user(self, obj):
+        if not hasattr(self, "_registered_user_cache"):
+            self._registered_user_cache = {}
+        if obj.pk not in self._registered_user_cache:
+            view = self.context.get("view")
+            organization = getattr(view, "organization", None)
+            org_reg_user = None
+            global_reg_user = None
+            # We iterate over .all() instead of using .filter() because callers
+            # of this serializer (e.g. validate_auth_token) prefetch
+            # "registered_users" via prefetch_related. Using .all() hits the
+            # in-memory prefetch cache (0 DB queries), whereas .filter() would
+            # bypass the cache and issue a new query every time.
+            for ru in obj.registered_users.all():
+                if organization and ru.organization_id == organization.pk:
+                    org_reg_user = ru
+                    break
+                elif ru.organization_id is None:
+                    global_reg_user = ru
+            self._registered_user_cache[obj.pk] = org_reg_user or global_reg_user
+        return self._registered_user_cache[obj.pk]
+
+    def get_is_verified(self, obj):
+        reg_user = self._get_registered_user(obj)
+        return reg_user.is_verified if reg_user else None
+
+    def get_method(self, obj):
+        reg_user = self._get_registered_user(obj)
+        return reg_user.method if reg_user else None

openwisp_radius/api/urls.py

@@ -77,6 +77,11 @@ def get_api_urls(api_views=None):
                 api_views.change_phone_number,
                 name="phone_number_change",
             ),
+            path(
+                "radius/organization/<slug:slug>/account/registration-method/",
+                api_views.update_registered_user_registration_method,
+                name="update_registered_user_registration_method",
+            ),
             path("radius/batch/", api_views.batch, name="batch"),
             path(
                 "radius/organization/<slug:slug>/batch/<uuid:pk>/pdf/",

openwisp_radius/api/utils.py

@@ -9,6 +9,7 @@
 
 Organization = load_model("openwisp_users", "Organization")
 OrganizationRadiusSettings = load_model("openwisp_radius", "OrganizationRadiusSettings")
+RegisteredUser = load_model("openwisp_radius", "RegisteredUser")
 
 
 class ErrorDictMixin(object):
@@ -30,8 +31,18 @@ def _needs_identity_verification(self, organization_filter_kwargs={}, org=None):
         except ObjectDoesNotExist:
             return app_settings.NEEDS_IDENTITY_VERIFICATION
 
-    def is_identity_verified_strong(self, user):
-        try:
-            return user.registered_user.is_identity_verified_strong
-        except ObjectDoesNotExist:
+    def is_identity_verified_strong(self, user, organization=None):
+        reg_user = None
+        global_reg_user = None
+        # We use all() to utilize the prefetch cache, otherwise
+        # it would cause an additional query to fetch the registered user
+        for ru in user.registered_users.all():
+            if organization and ru.organization_id == organization.pk:
+                reg_user = ru
+                break
+            elif ru.organization_id is None:
+                global_reg_user = ru
+        reg_user = reg_user or global_reg_user
+        if reg_user is None:
             return False
+        return reg_user.is_identity_verified_strong