verifies request.user e-mail whenever we attempt an implicit grant

Before, we would only verify the e-mail address if an implicit grant was about to be performed. Now even if no grant is performed, we still check the e-mail address on /users/roles/accept/. Note that we are still not doing any e-mail verification if there is a role associated to the user to maintain frictionless registration when a personal profile is created.
djaodjin · Jul 19, 2023 · e9002be · e9002be
1 parent 7bd41a5
commit e9002be
Show file tree

Hide file tree

Showing 9 changed files with 96 additions and 49 deletions.
diff --git a/pyproject.toml b/pyproject.toml
@@ -57,7 +57,8 @@ saas = [
   'templates/saas/metrics/*.html',
   'templates/saas/profile/*.html',
   'templates/saas/profile/roles/*.html',
-  'templates/saas/users/*.html'
+  'templates/saas/users/*.html',
+  'templates/saas/users/roles/*.html'
 ]
 
 [tool.setuptools.dynamic]

diff --git a/saas/api/accounts.py b/saas/api/accounts.py
@@ -27,7 +27,6 @@
 from rest_framework.generics import (ListAPIView, ListCreateAPIView,
     RetrieveAPIView)
 
-from .organizations import OrganizationQuerysetMixin
 from .serializers import (OrganizationSerializer, OrganizationCreateSerializer,
     OrganizationDetailSerializer)
 from .. import filters, settings
@@ -68,6 +67,16 @@ def get_order_func(fields):
         get_order_func(fields[1:])(left, right))
 
 
+class OrganizationQuerysetMixin(OrganizationDecorateMixin):
+
+    # Implementation Note: We are using this query in typeahead API calls.
+    # Typeaheads are more often than not used to request/grant access to
+    # a profile. We don't want to return profiles that cannot be notified
+    # of a request.
+    queryset = get_organization_model().objects.filter(
+        is_active=True, email__isnull=False).exclude(email="")
+
+
 class AccountsTypeaheadAPIView(OrganizationSmartListMixin,
                             OrganizationQuerysetMixin, ListAPIView):
     """

diff --git a/saas/models.py b/saas/models.py
@@ -169,7 +169,11 @@ def accessible_by(self, user, role_descr=None): # OrganizationManager
                     str(descr) for descr in role_descr]})
         roles = get_role_model().objects.db_manager(
             using=self._db).valid_for(**kwargs)
-        return self.filter(pk__in=roles.values('organization')).distinct()
+        return self.filter(
+            is_active=True, pk__in=roles.values('organization')).distinct()
+
+    def find_candidates_by_domain(self, domain):
+        return self.filter(is_active=True, email__endswith=domain)
 
     def find_candidates(self, full_name, user=None):
         """
@@ -2907,7 +2911,7 @@ class AdvanceDiscount(models.Model):
     discount_value = models.PositiveIntegerField(default=0,
         help_text=_('Amount of the discount'))
     length = models.PositiveSmallIntegerField(default=1,
-        help_text=_('Contract length associated with the period'))
+      help_text=_("Contract length associated with the period (defaults to 1)"))
 
     def __str__(self):
         return "%s-%s-%d" % (self.plan, slugify(

diff --git a/saas/templates/saas/users/roles/accept.html b/saas/templates/saas/users/roles/accept.html
@@ -0,0 +1,27 @@
+{% extends "saas/base.html" %}
+
+{% block content %}
+<section>
+  <div>
+    {% if role %}
+    <p>
+Based on your e-mail address we have granted you a {{role_descr}} role on {{organization}}.
+    </p>
+    <p>
+      {% if contacts %}
+If you need extra permissions, please contact one of the profile managers for {{organization}}: {{contacts}}.
+      {% else %}
+If you have any questions, please <a href="/contact/">contact us</a>.
+      {% endif %}
+    </p>
+    <div>
+      <a id="ok" href="{{next}}">Continue</a>
+    </div>
+    {% else %}
+    <p>
+We found a matching profile for your e-mail domain! Please verify your e-mail address before going further. To do so click on the link in the e-mail we just sent you.
+    </p>
+    {% endif %}
+  </div>
+</div>
+{% endblock %}
diff --git a/saas/templates/saas/users/roles.html → saas/templates/saas/users/roles/index.html b/saas/templates/saas/users/roles.html → saas/templates/saas/users/roles/index.html
diff --git a/saas/views/redirects.py b/saas/views/redirects.py
@@ -165,8 +165,9 @@ def get_natural_profile(self, request):
                 request.user, user, organization)
         else:
             user = request.user
-            organization = self.organization_model.objects.filter(
-                email__endswith=domain).get()
+            organization = \
+                self.organization_model.objects.find_candidates_by_domain(
+                domain).get()
         return user, organization
 
     def get(self, request, *args, **kwargs):

diff --git a/saas/views/roles.py b/saas/views/roles.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2021, DjaoDjin inc.
+# Copyright (c) 2023, DjaoDjin inc.
 # All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
@@ -26,13 +26,13 @@
 import logging
 
 from django import http
-from django.contrib import messages
 from django.contrib.auth import REDIRECT_FIELD_NAME, get_user_model
 from django.db.models import Q
-from django.views.generic.base import RedirectView
+from django.views.generic.base import (ContextMixin, TemplateResponseMixin,
+    RedirectView)
 
 from .. import settings
-from ..compat import gettext_lazy as _, reverse
+from ..compat import reverse
 from ..mixins import product_url
 from ..models import RoleDescription
 from ..utils import (get_organization_model, get_role_model,
@@ -41,7 +41,8 @@
 LOGGER = logging.getLogger(__name__)
 
 
-class RoleImplicitGrantAcceptView(RedirectView):
+class RoleImplicitGrantAcceptView(ContextMixin, TemplateResponseMixin,
+                                  RedirectView):
     """
     Accept implicit role on an organization if no role exists for the user.
     """
@@ -50,34 +51,27 @@ class RoleImplicitGrantAcceptView(RedirectView):
     role_model = get_role_model()
     user_model = get_user_model()
     organization_model = get_organization_model()
+    template_name = 'saas/users/roles/accept.html'
 
     def check_email_verified(self, request, user,
                              redirect_field_name=REDIRECT_FIELD_NAME,
                              next_url=None):
         #pylint:disable=unused-argument
         return True
 
+
     def get_implicit_grant_response(self, next_url, role, *args, **kwargs):
-        #pylint:disable=unused-argument
+        context = self.get_context_data(**kwargs)
+        context.update({REDIRECT_FIELD_NAME: next_url})
         if role:
-            organization = role.organization
-            role_descr = role.role_description
-            messages.info(self.request, _("Based on your e-mail address"\
-                " we have granted you a %(role_descr)s role on"\
-                " %(organization)s. If you need extra permissions,"\
-                " contact one of the profile managers for"\
-                " %(organization)s: %(managers)s.") % {
-                'role_descr': role_descr.title,
-                'organization': organization.printable_name,
-                'managers': ', '.join([user.get_full_name() for user
-                    in organization.with_role(settings.MANAGER).exclude(
-                        pk=self.request.user.pk)])})
-        else:
-            messages.info(self.request, _("You need to verify"\
-                " your e-mail address before going further. Please"\
-                " click on the link in the e-mail we just sent you."\
-                " Thank you."))
-        return http.HttpResponseRedirect(next_url)
+            context.update({
+                'role': role,
+                'contacts': ', '.join([user.get_full_name() for user
+                    in role.organization.with_role(settings.MANAGER).exclude(
+                    pk=self.request.user.pk)])
+            })
+        return self.render_to_response(context)
+
 
     def get_natural_profile(self, request):
         """
@@ -102,8 +96,9 @@ def get_natural_profile(self, request):
                 request.user, user, organization)
         else:
             user = request.user
-            organization = self.organization_model.objects.filter(
-                email__endswith=domain).get()
+            organization = \
+                self.organization_model.objects.find_candidates_by_domain(
+                domain).get()
         return user, organization
 
     def get_redirect_url(self, *args, **kwargs):
@@ -121,13 +116,20 @@ def get_redirect_url(self, *args, **kwargs):
 
     def get(self, request, *args, **kwargs):
         redirect_to = reverse('saas_user_product_list', args=(request.user,))
-        next_url = self.request.GET.get(REDIRECT_FIELD_NAME, None)
-        if next_url:
-            redirect_to += '?%s=%s' % (REDIRECT_FIELD_NAME, next_url)
+        next_to = validate_redirect_url(
+            self.request.GET.get(REDIRECT_FIELD_NAME, None))
+        if next_to:
+            redirect_to += '?%s=%s' % (REDIRECT_FIELD_NAME, next_to)
 
-        # We will attempt to assign the user to a profile.
-        # Find an organization with a matching e-mail domain.
+        # If there are no roles associated to the user, we will attempt
+        # to assign the user to a profile with a matching e-mail domain.
+        is_email_verified = True  # If there is a role, we don't force
+                                  # verification of the e-mail address.
         if not self.role_model.objects.filter(user=self.request.user).exists():
+            # In all cases, we verify the user has access to the e-mail account.
+            next_url = self.get_redirect_url(*args, **kwargs)
+            is_email_verified = self.check_email_verified(request, request.user,
+                next_url=next_url)
             # XXX copy/pasted from `OrganizationRedirectView`
             domain = request.user.email.split('@')[-1].lower()
             try:
@@ -146,9 +148,7 @@ def get(self, request, *args, **kwargs):
                             settings.MANAGER)
                     # Create a granted role implicitely, but only if the e-mail
                     # was verified.
-                    next_url = self.get_redirect_url(*args, **kwargs)
-                    if self.check_email_verified(request, user,
-                            next_url=next_url):
+                    if is_email_verified:
                         role = organization.add_role_request(
                             user, role_descr=role_descr)
                         if role.request_key:
@@ -163,9 +163,6 @@ def get(self, request, *args, **kwargs):
                         next_url = self.get_redirect_url(*args, **kwargs)
                         return self.get_implicit_grant_response(
                             next_url, role, *args, **kwargs)
-                    # We are redirecting because the e-mail must be verified
-                    return self.get_implicit_grant_response(
-                        redirect_to, None, *args, **kwargs)
                 except RoleDescription.DoesNotExist:
                     LOGGER.debug("'%s' does not have a role on any profile but"
                         " we cannot grant one implicitely because there is"
@@ -186,5 +183,9 @@ def get(self, request, *args, **kwargs):
                     " we cannot grant one implicitely because @%s is"
                     " ambiguous. Multiple profiles share that email domain.",
                     request.user, domain)
+        if not is_email_verified:
+            # We are redirecting because the e-mail must be verified
+            return self.get_implicit_grant_response(
+                redirect_to, None, *args, **kwargs)
         # XXX This one must return to users/roles/!!!
         return http.HttpResponseRedirect(redirect_to)
diff --git a/saas/views/users.py b/saas/views/users.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2020, DjaoDjin inc.
+# Copyright (c) 2023, DjaoDjin inc.
 # All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
@@ -36,8 +36,8 @@ class ProductListView(UserMixin, TemplateView):
     Template:
 
     To edit the layout of this page, create a local \
-    ``saas/users/roles.html`` (`example <https://github.com/djaodjin\
-/djaodjin-saas/tree/master/saas/templates/saas/users/roles.html>`__).
+    ``saas/users/roles/index.html`` (`example <https://github.com/djaodjin\
+/djaodjin-saas/tree/master/saas/templates/saas/users/roles/index.html>`__).
     You should insure the page will call the
     `/api/users/{user}/accessibles <https://www.djaodjin.com/docs/\
 reference/djaoapp/latest/api/#listAccessibleBy>`__
@@ -47,9 +47,13 @@ class ProductListView(UserMixin, TemplateView):
       - ``user`` The organization object users have permissions to.
       - ``request`` The HTTP request object
     """
-    # XXX We use ``OrganizationMixin`` so that urls.pricing is defined.
+    template_name = 'saas/users/roles/index.html'
 
-    template_name = 'saas/users/roles.html'
+    def get_template_names(self):
+        return super(ProductListView, self).get_template_names() + [
+            # Implementation note: backward compatibility
+            'saas/users/roles.html'
+        ]
 
     def get_context_data(self, **kwargs):
         context = super(ProductListView, self).get_context_data(**kwargs)

diff --git a/testsite/requirements.txt b/testsite/requirements.txt
@@ -1,4 +1,4 @@
-Django==3.2.19
+Django==3.2.20
 django-countries==7.2.1
 django-localflavor==3.1
 django-phonenumber-field==5.2.0