fix(FT OIDC): prevent overwriting duplicate emails

fix(PoleEmploiConnectUserData): raise EmailInUseException on duplicate emails

France Travail allows users to overload the same email, and we allow only one

feat(pe_connect_callback): catch EmailInUseException

feat: render messages as modals

fix: quality

feat(france_travail_registration_failure.html): redirect to jobseeker registration

fix: tests

feat(users/tests): test_get_redacted_full_name

fix: solution should work for other identity providers

refactor(User.get_redacted_full_name): extend existing utility

fix(pe_connect/views.py): security best practice

fix: remove unused code

fix: requested changes

refactor: move modals messages to modals block

itou/openid_connect/models.py

@@ -11,6 +11,12 @@
 from .constants import OIDC_STATE_CLEANUP, OIDC_STATE_EXPIRATION
 
 
+class EmailInUseException(Exception):
+    def __init__(self, user, *args):
+        self.user = user
+        super().__init__(*args)
+
+
 class InvalidKindException(Exception):
     def __init__(self, user, *args):
         self.user = user
@@ -109,8 +115,9 @@ def create_or_update_user(self, is_login=False):
         Create or update a user managed by another identity provider.
          - If there is already a user with this username (user_info_dict["sub"])
            and from the same identity provider, we update and return it.
-         - If there is already a user with the email, we return this user.
-         - otherwise, we create a new user based on the data we received.
+         - If there is already a user with the email, we return this user, provided
+           the email is not in use by another account of the same provider (some providers allow email overloading).
+         - Otherwise, we create a new user based on the data we received.
         """
         user_data_dict = dataclasses.asdict(self)
         user_data_dict = {key: value for key, value in user_data_dict.items() if value}
@@ -130,6 +137,8 @@ def create_or_update_user(self, is_login=False):
                     self.check_valid_kind(user, user_data_dict, is_login)
                     # Don't update a user handled by another SSO provider.
                     return user, created
+                if user.identity_provider == self.identity_provider:
+                    raise EmailInUseException(user)
             except User.DoesNotExist:
                 # User.objects.create_user does the following:
                 # - set User.is_active to true,

itou/openid_connect/pe_connect/views.py

@@ -9,6 +9,7 @@
 from django.shortcuts import render
 from django.urls import reverse
 from django.utils import crypto
+from django.utils.html import format_html
 from django.utils.http import urlencode
 
 from itou.external_data.models import ExternalDataImport
@@ -17,17 +18,17 @@
 from itou.utils import constants as global_constants
 from itou.utils.urls import add_url_params, get_absolute_url
 
-from ..models import InvalidKindException, MultipleUsersFoundException
+from ..models import EmailInUseException, InvalidKindException, MultipleUsersFoundException
 from . import constants
 from .models import PoleEmploiConnectState, PoleEmploiConnectUserData
 
 
 logger = logging.getLogger(__name__)
 
 
-def _redirect_to_job_seeker_login_on_error(error_msg, request=None):
+def _redirect_to_job_seeker_login_on_error(error_msg, request=None, extra_tags=""):
     if request:
-        messages.error(request, error_msg)
+        messages.error(request, error_msg, extra_tags)
     return HttpResponseRedirect(reverse("login:job_seeker"))
 
 
@@ -139,12 +140,32 @@ def pe_connect_callback(request):
         return HttpResponseRedirect(url)
     except MultipleUsersFoundException as e:
         return _redirect_to_job_seeker_login_on_error(
-            "Vous avez deux comptes sur la plateforme et nous détectons un conflit d'email : "
-            f"{e.users[0].email} et {e.users[1].email}. "
-            "Veuillez vous rapprocher du support pour débloquer la situation en suivant "
-            f"<a href='{global_constants.ITOU_HELP_CENTER_URL}'>ce lien</a>.",
+            format_html(
+                "Vous avez deux comptes sur la plateforme et nous détectons un conflit d'email : "
+                "{} et {}. "
+                "Veuillez vous rapprocher du support pour débloquer la situation en suivant "
+                "<a href='{}'>ce lien</a>.",
+                e.users[0].email,
+                e.users[1].email,
+                global_constants.ITOU_HELP_CENTER_URL,
+            ),
             request=request,
         )
+    except EmailInUseException as e:
+        redacted_name = e.user.get_redacted_full_name()
+        msg_who = f" au nom de <strong>{redacted_name}</strong>" if redacted_name else ""
+
+        return _redirect_to_job_seeker_login_on_error(
+            format_html(
+                "Vous avez essayé de vous connecter avec un compte France Travail, mais un compte"
+                "{} a déjà été créé avec cette adresse e-mail. Vous pouvez néanmoins"
+                " vous inscrire en utilisant une autre adresse e-mail et un mot de passe"
+                ' en cliquant sur <strong>"Inscription"</strong>.',
+                msg_who,
+            ),
+            request=request,
+            extra_tags="modal france_travail_registration_failure",
+        )
 
     nir = request.session.get(global_constants.ITOU_SESSION_NIR_KEY)
     if nir:

itou/templates/django_bootstrap5/messages.html

@@ -1,6 +1,6 @@
 {% load i18n django_bootstrap5 %}
 {% for message in messages %}
-    {% if "toast" not in message.tags %}
+    {% if "toast" not in message.tags and "modal" not in message.tags %}
         <div class="alert alert-{{ message|bootstrap_message_alert_type }} alert-dismissible fade show" role="alert">
             <button type="button" class="btn-close" data-bs-dismiss="alert" aria-label="{% trans 'close' %}"></button>
             {{ message }}

itou/templates/layout/base.html

@@ -264,6 +264,8 @@
             {% include "layout/_news_modal.html" %}
         {% endif %}
 
-        {% block modals %}{% endblock %}
+        {% block modals %}
+            {% include "utils/modals.html" %}
+        {% endblock %}
     </body>
 </html>

itou/templates/utils/modal_includes/france_travail_registration_failure.html

@@ -0,0 +1,23 @@
+{% load static %}
+{% load theme_inclusion %}
+{% load django_bootstrap5 %}
+
+<div class="modal-dialog modal-dialog-centered">
+    <div class="modal-content">
+        <div class="modal-header">
+            <h3 class="modal-title text-danger" id="message-modal-{{ forloop.counter }}-label">
+                <i class="ri-xl ri-alert-line"></i>
+                L'inscription a échoué
+            </h3>
+            <button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Fermer"></button>
+        </div>
+        <div class="row modal-body">
+            <h2 class="h4">L'inscription via France Travail a échoué</h2>
+            <p>{{ message }}</p>
+        </div>
+        <div class="modal-footer">
+            <button type="button" class="btn" data-bs-dismiss="modal">Retour</button>
+            <a href="{% url 'signup:job_seeker' %}" class="btn btn-primary">Inscription</a>
+        </div>
+    </div>
+</div>

itou/templates/utils/modals.html

@@ -0,0 +1,19 @@
+{% load static %}
+{% load theme_inclusion %}
+{% load django_bootstrap5 %}
+
+{% for message in messages %}
+    {% if "modal" in message.extra_tags %}
+        <div class="modal fade" id="message-modal-{{ forloop.counter }}" data-bs-backdrop="static" tabindex="-1" role="dialog" aria-labelledby="message-modal-{{ forloop.counter }}-label" aria-modal="true">
+            {% if "france_travail_registration_failure" in message.extra_tags %}
+                {% include "utils/modal_includes/france_travail_registration_failure.html" %}
+            {% endif %}
+        </div>
+
+        <script nonce="{{ CSP_NONCE }}">
+            window.onload = function() {
+                new bootstrap.Modal(document.getElementById("message-modal-{{ forloop.counter }}")).show();
+            };
+        </script>
+    {% endif %}
+{% endfor %}

itou/users/models.py

@@ -34,6 +34,7 @@
 from itou.companies.enums import CompanyKind
 from itou.utils.db import or_queries
 from itou.utils.models import UniqueConstraintWithErrorCode
+from itou.utils.templatetags.str_filters import mask_unless
 from itou.utils.validators import validate_birthdate, validate_nir, validate_pole_emploi_id
 
 from .enums import IdentityProvider, LackOfNIRReason, LackOfPoleEmploiId, Title, UserKind
@@ -356,6 +357,19 @@ def get_full_name(self):
         full_name = f"{self.first_name.strip().title()} {self.last_name.upper()}"
         return full_name.strip()[:70]
 
+    def get_redacted_full_name(self):
+        """
+        Return full name in redacted form for privacy, e.g. J** H***h
+        """
+
+        def get_mask_for_part(part):
+            visible_chars = 1 if len(part) <= 3 else 2
+            last_char = part[-1] if visible_chars > 1 else ""
+            return part[0] + "*" * (min(len(part) - visible_chars, 10)) + last_char
+
+        # we don't use get_full_name here to limit the transformations applied to the user's names in this context
+        return mask_unless(f"{self.first_name.title()} {self.last_name.title()}", False, get_mask_for_part)
+
     @property
     def is_job_seeker(self):
         return self.kind == UserKind.JOB_SEEKER

itou/utils/templatetags/str_filters.py

@@ -32,8 +32,8 @@ def pluralizefr(value, arg="s"):
 
 @register.filter(is_safe=True)
 @defaultfilters.stringfilter
-def mask_unless(value, predicate):
+def mask_unless(value, predicate, mask_function=(lambda x: x[0] + "…")):
     if predicate:
         return value
 
-    return " ".join(part[0] + "…" for part in re.split(f"[{re.escape(string.whitespace)}]+", value) if part)
+    return " ".join(mask_function(part) for part in re.split(f"[{re.escape(string.whitespace)}]+", value) if part)

tests/openid_connect/pe_connect/tests.py

@@ -15,7 +15,7 @@
 from itou.external_data.apis import pe_connect
 from itou.external_data.models import ExternalDataImport
 from itou.openid_connect.constants import OIDC_STATE_CLEANUP
-from itou.openid_connect.models import InvalidKindException
+from itou.openid_connect.models import EmailInUseException, InvalidKindException
 from itou.openid_connect.pe_connect import constants
 from itou.openid_connect.pe_connect.models import PoleEmploiConnectState, PoleEmploiConnectUserData
 from itou.users.enums import IdentityProvider, UserKind
@@ -181,6 +181,23 @@ def test_create_user_with_already_existing_peamu_id_but_from_other_sso(self):
         with pytest.raises(ValidationError):
             peamu_user_data.create_or_update_user()
 
+    def test_create_user_with_already_existing_peamu_email(self):
+        """
+        If there already is an existing user from PE Connect with this email,
+        but under another username, we raise an error.
+
+        We do this because France Travail allows users to overload the same email
+        across multiple accounts, where we only allow one.
+        """
+        peamu_user_data = PoleEmploiConnectUserData.from_user_info(PEAMU_USERINFO)
+        JobSeekerFactory(
+            username="another_username",
+            email=peamu_user_data.email,
+            identity_provider=IdentityProvider.PE_CONNECT,
+        )
+        with pytest.raises(EmailInUseException):
+            peamu_user_data.create_or_update_user()
+
     def test_create_django_user_with_already_existing_peamu_email_django(self):
         """
         If there already is an existing user from Django with email PEAMU sent us

tests/users/test_models.py

@@ -661,6 +661,29 @@ def test_get_full_name(self):
 
         assert len(JobSeekerFactory(first_name=too_long_name, last_name="maréchal").get_full_name()) == 70
 
+    def test_get_redacted_full_name(self):
+        user = JobSeekerFactory(first_name="", last_name="Bach")
+
+        def override_full_name(full_name):
+            names = full_name.split(" ")
+            user.first_name = " ".join(names[:-1]) if len(names) > 1 else names[0]
+            user.last_name = names[-1] if len(names) > 1 else ""
+
+        test_names = [
+            ("Johan Sebastian Bach", "J***n S*******n B**h"),
+            ("Max Weber", "M** W***r"),
+            ("Charlemagne", "C*********e"),
+            ("Salvador Felipe Jacinto Dalí y Domenech", "S******r F****e J*****o D**í Y D******h"),
+            ("Harald Blue-tooth", "H****d B********h"),
+            ("Llanfairpwllgwyngyllgogerychwyrndrobwllllantysiliogogogoch", "L**********h"),
+            ("Max", "M**"),
+            ("", ""),
+        ]
+
+        for name, expected_output in test_names:
+            override_full_name(name)
+            assert user.get_redacted_full_name() == expected_output
+
 
 class JobSeekerProfileModelTest(TestCase):
     def setUp(self):

tests/www/invitations_views/test_company_accept.py

@@ -169,7 +169,11 @@ def test_accept_invitation_signup_bad_email_case(self):
     @respx.mock
     def test_accept_existing_user_not_logged_in_using_IC(self):
         invitation = SentEmployerInvitationFactory(email=OIDC_USERINFO["email"])
-        user = EmployerFactory(email=OIDC_USERINFO["email"], has_completed_welcoming_tour=True)
+        user = EmployerFactory(
+            username=OIDC_USERINFO["sub"],
+            email=OIDC_USERINFO["email"],
+            has_completed_welcoming_tour=True,
+        )
         response = self.client.get(invitation.acceptance_link, follow=True)
         assert reverse("login:employer") in response.wsgi_request.get_full_path()
         assert not invitation.accepted

tests/www/invitations_views/test_prescriber_organization.py

@@ -409,7 +409,11 @@ def test_accept_existing_user_belongs_to_another_organization(self):
     @respx.mock
     def test_accept_existing_user_not_logged_in_using_IC(self):
         invitation = PrescriberWithOrgSentInvitationFactory(sender=self.sender, organization=self.organization)
-        user = PrescriberFactory(email=OIDC_USERINFO["email"], has_completed_welcoming_tour=True)
+        user = PrescriberFactory(
+            username=OIDC_USERINFO["sub"],
+            email=OIDC_USERINFO["email"],
+            has_completed_welcoming_tour=True,
+        )
         # The user verified its email
         EmailAddress(user_id=user.pk, email=user.email, verified=True, primary=True).save()
         invitation = PrescriberWithOrgSentInvitationFactory(

tests/www/signup/test_prescriber.py

@@ -737,7 +737,7 @@ def test_prescriber_already_exists__simple_signup(self):
         He will be logged in instead as if he just used the login through IC button
         """
         #### User is a prescriber. Update it and connect. ####
-        PrescriberFactory(email=OIDC_USERINFO["email"])
+        PrescriberFactory(email=OIDC_USERINFO["email"], username=OIDC_USERINFO["sub"])
         self.client.get(reverse("signup:prescriber_check_already_exists"))
 
         # Step 2: register as a simple prescriber (orienteur).
@@ -772,7 +772,7 @@ def test_prescriber_already_exists__create_organization(self):
         We should update his account and make him join this new organization.
         """
         org = PrescriberOrganizationFactory.build(kind=PrescriberOrganizationKind.OTHER)
-        user = PrescriberFactory(email=OIDC_USERINFO["email"])
+        user = PrescriberFactory(email=OIDC_USERINFO["email"], username=OIDC_USERINFO["sub"])
 
         self.client.get(reverse("signup:prescriber_check_already_exists"))
 
@@ -886,7 +886,7 @@ def test_organization_creation_error(self):
         The user is still created and can try again.
         """
         org = PrescriberOrganizationFactory(kind=PrescriberOrganizationKind.OTHER)
-        user = PrescriberFactory(email=OIDC_USERINFO["email"])
+        user = PrescriberFactory(email=OIDC_USERINFO["email"], username=OIDC_USERINFO["sub"])
 
         self.client.get(reverse("signup:prescriber_check_already_exists"))
 
@@ -958,7 +958,7 @@ def test_non_prescriber_cant_join_organisation(self):
         The user is still created and can try again.
         """
         org = PrescriberOrganizationFactory(kind=PrescriberOrganizationKind.OTHER)
-        EmployerFactory(email=OIDC_USERINFO["email"], with_company=True)
+        EmployerFactory(email=OIDC_USERINFO["email"], username=OIDC_USERINFO["sub"], with_company=True)
 
         response = self.client.get(reverse("signup:prescriber_check_already_exists"))
         assert response.status_code == 200
@@ -1034,7 +1034,7 @@ def test_employer_already_exists(self):
         Raise an exception.
         """
         org = PrescriberOrganizationFactory.build(kind=PrescriberOrganizationKind.OTHER)
-        user = EmployerFactory(email=OIDC_USERINFO["email"])
+        user = EmployerFactory(email=OIDC_USERINFO["email"], username=OIDC_USERINFO["sub"])
         self.client.get(reverse("signup:prescriber_check_already_exists"))
 
         session_signup_data = self.client.session.get(global_constants.ITOU_SESSION_PRESCRIBER_SIGNUP_KEY)

tests/www/signup/test_siae.py

@@ -127,7 +127,9 @@ def test_join_an_company_without_members_as_an_existing_employer(self):
         company = CompanyFactory(kind=CompanyKind.ETTI)
         assert 0 == company.members.count()
 
-        user = EmployerFactory(email=OIDC_USERINFO["email"], has_completed_welcoming_tour=True)
+        user = EmployerFactory(
+            username=OIDC_USERINFO["sub"], email=OIDC_USERINFO["email"], has_completed_welcoming_tour=True
+        )
         CompanyMembershipFactory(user=user)
         assert 1 == user.company_set.count()
 
@@ -170,7 +172,9 @@ def test_join_an_company_without_members_as_an_existing_employer_returns_on_othe
         """
         company = CompanyFactory(kind=CompanyKind.ETTI)
 
-        user = EmployerFactory(email=OIDC_USERINFO["email"], has_completed_welcoming_tour=True)
+        user = EmployerFactory(
+            username=OIDC_USERINFO["sub"], email=OIDC_USERINFO["email"], has_completed_welcoming_tour=True
+        )
         CompanyMembershipFactory(user=user)
 
         magic_link = company.signup_magic_link