Only accept project invitations whose email match new user email (#2449)

* remove project invitation id and token verification

remove invitation_id and invitation_token query params from invitation email link.
remove support for allowing a user to register using a different email from the one the invite was sent to
add a post_save signal to accept only invitations that match the new user email and remove implementation for accepting invitation from the UserProfileSerializer. This is because a user can also be created using OIDC

* update project invitation documentation

docs/projects.rst

@@ -638,7 +638,7 @@ Response
         }
     
 
-The link embedded in the email will be of the format ``http://{url}?invitation_id={id}&invitation_token={token}`` 
+The link embedded in the email will be of the format ``http://{url}`` 
 where:
 
 - ``url`` - is the URL the recipient will be redirected to on clicking the link. The default is ``{domain}/api/v1/profiles`` where ``domain`` is domain where the API is hosted.
@@ -652,9 +652,6 @@ adding the setting ``PROJECT_INVITATION_URL``
 
     PROJECT_INVITATION_URL = 'https://example.com/register'
 
-- ``id`` - The ``ProjectInvitation`` object primary key encoded to base 64
-- ``token`` - is a hash value that will be used to confirm validity of the link.
-
 
 Update a project invitation
 ---------------------------
@@ -751,28 +748,7 @@ Accept a project invitation
 ---------------------------
 
 Since a project invitation is sent to an unregistered user, acceptance of the invitation is handled
-when creating a new user.
-
-The ``invitation_id`` and ``invitation_token`` query params are added to 
-the `create user <https://github.com/onaio/onadata/blob/main/docs/profiles.rst#register-a-new-user>`_ endpoint
-
-where:
-
-- ``id`` - is the value of the project ``invitation_id`` query parameter from the url embedded in the project invitation email
-- ``token`` - is the value of the project ``invitation_token`` query parameter from the url embedded in the project invitation email
-
-.. raw:: html
-
-	<pre class="prettyprint"><b>POST</b> /api/v1/profiles?invitation_id={id}&invitation_token={token}</pre>
-
-
-The validation of the ``id`` and ``token`` are dependent on one another and both should be provided for 
-successful validation. Failure of validation does not prevent the account creation. However, the new
-user will not have the projects shared with them.
-
-If the validation for ``id`` and ``token`` is successful:
-
-- The invitation will be accepted including any other pending invitations whose emails match the invitation's email.
-- If the invitation's email matches the new user's email, the new user's will immediately be marked as verified.
+when `creating a new user <https://github.com/onaio/onadata/blob/main/docs/profiles.rst#register-a-new-user>`_.
 
-If ``id`` and ``token`` are invalid or are not provided but the user registers using an email that matches a pending invitation, then that project is shared with the user.
+All pending invitations whose email match the new user's email will be accepted and projects shared with the
+user

onadata/apps/api/tasks.py

@@ -6,7 +6,6 @@
 import sys
 import logging
 from datetime import timedelta
-from typing import Optional
 
 from celery.result import AsyncResult
 from django.conf import settings
@@ -21,7 +20,6 @@
 from onadata.libs.utils.model_tools import queryset_iterator
 from onadata.apps.logger.models import Instance, ProjectInvitation, XForm
 from onadata.libs.utils.email import ProjectInvitationEmail
-from onadata.libs.utils.user_auth import accept_project_invitation
 from onadata.celeryapp import app
 
 User = get_user_model()
@@ -147,24 +145,3 @@ def send_project_invitation_email_async(
     else:
         email = ProjectInvitationEmail(invitation, url)
         email.send()
-
-
-@app.task()
-def accept_project_invitation_async(user_id: str, invitation_id: Optional[str] = None):
-    """Accpet project invitation asynchronously"""
-    invitation = None
-
-    if invitation_id:
-        try:
-            invitation = ProjectInvitation.objects.get(id=invitation_id)
-        except ProjectInvitation.DoesNotExist as err:
-            logging.exception(err)
-
-    try:
-        user = User.objects.get(pk=user_id)
-
-    except User.DoesNotExist as err:
-        logging.exception(err)
-
-    else:
-        accept_project_invitation(user, invitation)

onadata/apps/api/tests/test_tasks.py

@@ -5,7 +5,6 @@
 from onadata.apps.main.tests.test_base import TestBase
 from onadata.apps.api.tasks import (
     send_project_invitation_email_async,
-    accept_project_invitation_async,
 )
 from onadata.apps.logger.models import ProjectInvitation
 from onadata.libs.utils.user_auth import get_user_default_project
@@ -31,28 +30,3 @@ def test_sends_email(self, mock_send):
         url = "https://example.com/register"
         send_project_invitation_email_async(self.invitation.id, url)
         mock_send.assert_called_once()
-
-
-@patch("onadata.apps.api.tasks.accept_project_invitation")
-class AcceptProjectInvitationTesCase(TestBase):
-    """Tests for accept_project_invitation_async"""
-
-    def setUp(self):
-        super().setUp()
-
-        project = get_user_default_project(self.user)
-        self.invitation = ProjectInvitation.objects.create(
-            project=project,
-            email="[email protected]",
-            role="manager",
-        )
-
-    def test_accept_invitation(self, mock_accept_invitation):
-        """Test invitation is accepted"""
-        accept_project_invitation_async(self.user.id, self.invitation.id)
-        mock_accept_invitation.assert_called_once_with(self.user, self.invitation)
-
-    def test_invitation_id_optional(self, mock_accept_invitation):
-        """invitation_id argument is optional"""
-        accept_project_invitation_async(self.user.id)
-        mock_accept_invitation.assert_called_once_with(self.user, None)

onadata/apps/api/tests/viewsets/test_user_profile_viewset.py

@@ -7,7 +7,6 @@
 import json
 import os
 from six.moves.urllib.parse import urlparse, parse_qs
-from mock import Mock
 
 from django.contrib.auth import get_user_model
 from django.core.cache import cache
@@ -32,7 +31,7 @@
 from onadata.apps.main.models.user_profile import set_kpi_formbuilder_permissions
 from onadata.libs.authentication import DigestAuthentication
 from onadata.libs.serializers.user_profile_serializer import _get_first_last_names
-from onadata.libs.utils.email import ProjectInvitationEmail
+from onadata.libs.permissions import EditorRole
 
 
 User = get_user_model()
@@ -294,19 +293,13 @@ def test_profile_create(self, mock_send_verification_email):
 
     @override_settings(CELERY_TASK_ALWAYS_EAGER=True)
     @override_settings(ENABLE_EMAIL_VERIFICATION=True)
-    @patch(
-        (
-            "onadata.libs.serializers.user_profile_serializer."
-            "accept_project_invitation_async.delay"
-        )
-    )
     @patch(
         (
             "onadata.libs.serializers.user_profile_serializer."
             "send_verification_email.delay"
         )
     )
-    def test_accept_invitaton(self, mock_send_email, mock_accept_invitation):
+    def test_accept_invitaton(self, mock_send_email):
         """An invitation is accepted successfuly"""
         self._project_create()
         invitation = ProjectInvitation.objects.create(
@@ -318,84 +311,19 @@ def test_accept_invitaton(self, mock_send_email, mock_accept_invitation):
         data = _profile_data()
         del data["name"]
         data["email"] = invitation.email
-
-        with patch.object(
-            ProjectInvitationEmail, "check_invitation", Mock(return_value=invitation)
-        ) as mock_check_invitation:
-            request = self.factory.post(
-                "/api/v1/profiles?invitation_id=id&invitation_token=token",
-                data=json.dumps(data),
-                content_type="application/json",
-                **self.extra,
-            )
-            response = self.view(request)
-            self.assertEqual(response.status_code, 201)
-            user = User.objects.get(username="deno")
-            mock_check_invitation.assert_called_once_with("id", "token")
-            mock_accept_invitation.assert_called_with(user.id, invitation.id)
-            # user email matches invitation email so no need to send
-            # verification email
-            mock_send_email.assert_not_called()
-            self.assertTrue(user.profile.metadata["is_email_verified"])
-
-        # user registers using a different email from invitation email
-        data["email"] = "[email protected]"
-        data["username"] = "nicki"
-
-        with patch.object(
-            ProjectInvitationEmail, "check_invitation", Mock(return_value=invitation)
-        ):
-            request = self.factory.post(
-                "/api/v1/profiles?invitation_id=some_valid_id&invitation_token=some_token",
-                data=json.dumps(data),
-                content_type="application/json",
-                **self.extra,
-            )
-            response = self.view(request)
-            self.assertEqual(response.status_code, 201)
-            user = User.objects.get(username=data["username"])
-            mock_accept_invitation.assert_called_with(user.id, invitation.id)
-            # user email does not match invitation email so we send email verification
-            mock_send_email.assert_called_once()
-            self.assertFalse(user.profile.metadata.get("is_email_verified", False))
-
-        # invitation_id and invitation_token missing
-        data["email"] = "[email protected]"
-        data["username"] = "jack"
-
-        with patch.object(
-            ProjectInvitationEmail, "check_invitation", Mock(return_value=invitation)
-        ):
-            request = self.factory.post(
-                "/api/v1/profiles",
-                data=json.dumps(data),
-                content_type="application/json",
-                **self.extra,
-            )
-            response = self.view(request)
-            self.assertEqual(response.status_code, 201)
-            user = User.objects.get(username=data["username"])
-            mock_accept_invitation.assert_called_with(user.id, None)
-            self.assertFalse(user.profile.metadata.get("is_email_verified", False))
-
-        # invalid invitation
-        data["email"] = "[email protected]"
-        data["username"] = "jude"
-
-        with patch.object(
-            ProjectInvitationEmail, "check_invitation", Mock(return_value=None)
-        ):
-            request = self.factory.post(
-                "/api/v1/profiles?invitation_id=some_valid_id&invitation_token=some_token",
-                data=json.dumps(data),
-                content_type="application/json",
-                **self.extra,
-            )
-            response = self.view(request)
-            self.assertEqual(response.status_code, 201)
-            user = User.objects.get(username=data["username"])
-            mock_accept_invitation.assert_called_with(user.id, None)
-            self.assertFalse(user.profile.metadata.get("is_email_verified", False))
+        request = self.factory.post(
+            "/api/v1/profiles",
+            data=json.dumps(data),
+            content_type="application/json",
+            **self.extra,
+        )
+        response = self.view(request)
+        self.assertEqual(response.status_code, 201)
+        user = User.objects.get(username="deno")
+        mock_send_email.assert_called_once()
+        invitation.refresh_from_db()
+        self.assertEqual(invitation.status, ProjectInvitation.Status.ACCEPTED)
+        self.assertTrue(EditorRole.user_has_role(user, self.project))
 
     def _create_user_using_profiles_endpoint(self, data):
         request = self.factory.post(

onadata/apps/api/viewsets/user_profile_viewset.py

@@ -206,18 +206,6 @@ def get_object(self, queryset=None):
 
         return obj
 
-    def get_serializer(self, *args, **kwargs):
-        """Override get_serializer"""
-        if (invitation_id := self.request.query_params.get("invitation_id")) and (
-            invitation_token := self.request.query_params.get("invitation_token")
-        ):
-            draft_request_data = self.request.data.copy()
-            draft_request_data["invitation_id"] = invitation_id
-            draft_request_data["invitation_token"] = invitation_token
-            kwargs["data"] = draft_request_data
-
-        return super().get_serializer(*args, **kwargs)
-
     def update(self, request, *args, **kwargs):
         """Update user in cache and db"""
         username = kwargs.get("user")

onadata/apps/main/signals.py

@@ -4,10 +4,13 @@
 """
 from django.conf import settings
 from django.contrib.auth import get_user_model
+from django.db.models.signals import post_save
+from django.dispatch import receiver
 from django.template.loader import render_to_string
+from django.utils import timezone
 
 from onadata.libs.utils.email import send_generic_email
-
+from onadata.apps.logger.models import ProjectInvitation
 
 User = get_user_model()
 
@@ -62,3 +65,26 @@ def send_activation_email(sender, instance=None, **kwargs):
                     send_generic_email(
                         instance.email, email, f"{deployment_name} account activated"
                     )
+
+
+@receiver(post_save, sender=User, dispatch_uid="accept_project_invitation")
+def accept_project_invitation(sender, instance=None, created=False, **kwargs):
+    """Accept project invitations that match user email"""
+    if created:
+        invitation_qs = ProjectInvitation.objects.filter(
+            email=instance.email,
+            status=ProjectInvitation.Status.PENDING,
+        )
+        now = timezone.now()
+        # ShareProject needs to be imported inline because otherwise we get
+        # django.core.exceptions.AppRegistryNotReady: Apps aren't loaded yet.
+        # pylint: disable=import-outside-toplevel
+        from onadata.libs.models.share_project import ShareProject
+
+        for invitation in invitation_qs:
+            ShareProject(
+                invitation.project,
+                instance.username,
+                invitation.role,
+            ).save()
+            invitation.accept(accepted_at=now, accepted_by=instance)