Skip to content

Commit

Permalink
Use EmailAddress for signup unicity
Browse files Browse the repository at this point in the history
  • Loading branch information
calummackervoy committed Nov 26, 2024
1 parent ed5dcd3 commit 6a40156
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 14 deletions.
10 changes: 6 additions & 4 deletions itou/openid_connect/inclusion_connect/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import httpx
from allauth.account.adapter import get_adapter
from allauth.account.models import EmailAddress
from django.conf import settings
from django.contrib import messages
from django.contrib.auth import login
Expand All @@ -14,7 +15,6 @@

from itou.prescribers.models import PrescriberOrganization
from itou.users.enums import KIND_EMPLOYER, KIND_PRESCRIBER, IdentityProvider, UserKind
from itou.users.models import User
from itou.utils import constants as global_constants
from itou.utils.constants import ITOU_HELP_CENTER_URL
from itou.utils.urls import add_url_params, get_absolute_url
Expand Down Expand Up @@ -173,13 +173,15 @@ def inclusion_connect_activate_account(request):
return HttpResponseRedirect(params.get("previous_url", reverse("search:employers_home")))

user_kind = params.get("user_kind")
user = User.objects.filter(email=email).first()
email_address = EmailAddress.objects.filter(email=email).first()

if not user:
if not email_address:
params["register"] = True
request.GET = params
return inclusion_connect_authorize(request)

user = email_address.user

if user.kind != user_kind:
_add_user_kind_error_message(request, user, request.GET.get("user_kind"))
return HttpResponseRedirect(params.get("previous_url", reverse("search:employers_home")))
Expand Down Expand Up @@ -289,7 +291,7 @@ def inclusion_connect_callback(request):
try:
user, _ = ic_user_data.create_or_update_user(is_login=ic_state.data.get("is_login"))
except InvalidKindException:
existing_user = User.objects.get(email=user_data["email"])
existing_user = EmailAddress.objects.get(email=user_data["email"]).user
_add_user_kind_error_message(request, existing_user, user_kind)
is_successful = False
except MultipleSubSameEmailException as e:
Expand Down
10 changes: 7 additions & 3 deletions itou/openid_connect/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from typing import ClassVar
from urllib.parse import unquote

from allauth.account.models import EmailAddress
from django.core import signing
from django.db import models
from django.utils import crypto, timezone
Expand Down Expand Up @@ -162,14 +163,14 @@ def create_or_update_user(self, is_login=False):
except User.DoesNotExist:
try:
# A different user has already claimed this email address (we require emails to be unique)
user = User.objects.get(email=self.email)
user = EmailAddress.objects.get(email=self.email).user
if user.identity_provider == self.identity_provider:
if not self.allow_sub_update:
raise MultipleSubSameEmailException(user)
elif user.identity_provider not in self.allowed_identity_provider_migration:
self.check_valid_kind(user, user_data_dict, is_login)
raise EmailInUseException(user)
except User.DoesNotExist:
except EmailAddress.DoesNotExist:
# User.objects.create_user does the following:
# - set User.is_active to true,
# - call User.set_unusable_password() if no password is given.
Expand All @@ -183,7 +184,7 @@ def create_or_update_user(self, is_login=False):
user.jobseeker_profile.birthdate = birthdate
user.jobseeker_profile.save(update_fields={"birthdate"})
else:
other_user = User.objects.exclude(pk=user.pk).filter(email=self.email).first()
other_user = EmailAddress.objects.exclude(user=user).filter(email=self.email).first()
if other_user:
# We found a user with its sub, but there's another user using its email.
# This happens when the user tried to update its email with one already used by another account.
Expand All @@ -206,6 +207,9 @@ def create_or_update_user(self, is_login=False):
user.update_external_data_source_history_field(provider=self.identity_provider, field=key, value=value)
user.save()

# Cancel any ongoing email modifications for the user
EmailAddress.objects.filter(user=user).exclude(email=self.email).delete()

return user, created

@staticmethod
Expand Down
4 changes: 2 additions & 2 deletions itou/openid_connect/pro_connect/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import httpx
import jwt
from allauth.account.adapter import get_adapter
from allauth.account.models import EmailAddress
from django.conf import settings
from django.contrib import messages
from django.contrib.auth import login
Expand All @@ -15,7 +16,6 @@

from itou.prescribers.models import PrescriberOrganization
from itou.users.enums import KIND_EMPLOYER, KIND_PRESCRIBER, IdentityProvider, UserKind
from itou.users.models import User
from itou.utils import constants as global_constants
from itou.utils.constants import ITOU_HELP_CENTER_URL
from itou.utils.urls import add_url_params, get_absolute_url
Expand Down Expand Up @@ -242,7 +242,7 @@ def pro_connect_callback(request):
try:
user, _ = pc_user_data.create_or_update_user(is_login=pro_connect_state.data.get("is_login"))
except InvalidKindException:
existing_user = User.objects.get(email=user_data["email"])
existing_user = EmailAddress.objects.get(email=user_data["email"]).user
_add_user_kind_error_message(request, existing_user, user_kind)
is_successful = False
except EmailInUseException as e:
Expand Down
8 changes: 4 additions & 4 deletions itou/www/itou_staff_views/forms.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import datetime

from allauth.account.models import EmailAddress
from django import forms
from django.core.exceptions import ValidationError
from django.core.validators import MaxValueValidator
from django.utils import timezone

from itou.common_apps.address.departments import DEPARTMENTS
from itou.users.models import User
from itou.utils.widgets import DuetDatePickerWidget


Expand Down Expand Up @@ -58,10 +58,10 @@ class MergeUserForm(forms.Form):
)

def _check_email_exists(self, email):
user = User.objects.filter(email=email).first()
if not user:
email_address = EmailAddress.objects.filter(email=email).first()
if not email_address:
raise ValidationError("Cet utilisateur n'existe pas.")
return user
return email_address.user

def clean_email_1(self):
self.user_1 = self._check_email_exists(self.cleaned_data["email_1"])
Expand Down
3 changes: 2 additions & 1 deletion itou/www/signup/forms.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from allauth.account.adapter import get_adapter
from allauth.account.forms import SignupForm
from allauth.account.models import EmailAddress
from django import forms
from django.core.exceptions import ValidationError
from django.db.models.fields import BLANK_CHOICE_DASH
Expand Down Expand Up @@ -147,7 +148,7 @@ def clean_email(self):
raise ValidationError("Vous ne pouvez pas utiliser un e-mail Pôle emploi pour un candidat.")
if email.endswith(global_constants.FRANCE_TRAVAIL_EMAIL_SUFFIX):
raise ValidationError("Vous ne pouvez pas utiliser un e-mail France Travail pour un candidat.")
if User.objects.filter(email=email).exists():
if EmailAddress.objects.filter(email=email).exists():
raise ValidationError("Un autre utilisateur utilise déjà cette adresse e-mail.")
return email

Expand Down
24 changes: 24 additions & 0 deletions tests/openid_connect/pro_connect/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import httpx
import pytest
import respx
from allauth.account.models import EmailAddress
from django.contrib import auth, messages
from django.contrib.auth import get_user
from django.contrib.messages import Message
Expand Down Expand Up @@ -219,6 +220,29 @@ def test_get_existing_user_with_same_email_django(self):
assert user.username == OIDC_USERINFO["sub"]
assert user.identity_provider == users_enums.IdentityProvider.PRO_CONNECT

def test_get_existing_user_with_same_email_django_during_email_change(self):
"""
A variation of the situation where a Django user has reserved the email ProConnect authorized.
The email in question is not (yet) the primary email of this user - they are in the process of verifying it.
"""
pc_user_data = ProConnectPrescriberData.from_user_info(OIDC_USERINFO)
existing_user = PrescriberFactory(identity_provider=users_enums.IdentityProvider.DJANGO)
old_email = existing_user.email
existing_user.emailaddress_set.create(email=pc_user_data.email, primary=False, verified=False)
user, created = pc_user_data.create_or_update_user()
assert not created
assert user.last_name == OIDC_USERINFO["usual_name"]
assert user.first_name == OIDC_USERINFO["given_name"]
assert user.username == OIDC_USERINFO["sub"]
assert user.identity_provider == users_enums.IdentityProvider.PRO_CONNECT

# The user now has this email and only this email
# SSO users are not able to modify their email address via our site
assert user.email == pc_user_data.email
assert user.emailaddress_set.count() == 1
assert EmailAddress.objects.filter(email=pc_user_data.email, user=user).exists()
assert not EmailAddress.objects.filter(email=old_email).exists()

def test_get_existing_user_with_same_email_IC(self):
"""
If there already is an existing IC user with email ProConnect sent us, we do not create it again,
Expand Down

0 comments on commit 6a40156

Please sign in to comment.