Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Connexion : Empêcher les utilisateurs de se connecter à un autre compte du SSO partageant un courriel existant #4810

Merged
merged 1 commit into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions itou/openid_connect/france_connect/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,24 @@
from django.http import HttpResponseRedirect, JsonResponse
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.users.enums import UserKind
from itou.utils import constants as global_constants
from itou.utils.urls import get_absolute_url

from ..models import InvalidKindException, MultipleUsersFoundException
from ..models import EmailInUseException, InvalidKindException, MultipleUsersFoundException
from . import constants
from .models import FranceConnectState, FranceConnectUserData


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"))


Expand Down Expand Up @@ -134,6 +135,28 @@ def france_connect_callback(request):
UserKind.LABOR_INSPECTOR: reverse("login:labor_inspector"),
}[e.user.kind]
return HttpResponseRedirect(url)
except EmailInUseException as e:
redacted_name = e.user.get_redacted_full_name()
msg_who = (
format_html(
" au nom de <strong>{}</strong>",
redacted_name,
)
if redacted_name
else ""
)

return _redirect_to_job_seeker_login_on_error(
format_html(
"Vous avez essayé de vous connecter avec un compte France Connect, 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 sso_email_conflict_registration_failure",
)
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 : "
Expand Down
23 changes: 22 additions & 1 deletion itou/openid_connect/inclusion_connect/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from itou.utils.constants import ITOU_HELP_CENTER_URL
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 .enums import InclusionConnectChannel
from .models import (
Expand Down Expand Up @@ -275,6 +275,27 @@ def inclusion_connect_callback(request):
existing_user = User.objects.get(email=user_data["email"])
_add_user_kind_error_message(request, existing_user, user_kind)
is_successful = False
except EmailInUseException as e:
redacted_name = e.user.get_redacted_full_name()
msg_who = (
format_html(
" au nom de <strong>{}</strong>",
redacted_name,
)
if redacted_name
else ""
)

error = format_html(
"Vous avez essayé de vous connecter avec un compte Inclusion Connect, mais un compte"
"{} a déjà été créé avec cette adresse e-mail. "
"Veuillez vous rapprocher du support pour débloquer la situation en suivant "
"<a href='{}'>ce lien</a>.",
msg_who,
global_constants.ITOU_HELP_CENTER_URL,
)
messages.error(request, error)
is_successful = False
except MultipleUsersFoundException as e:
# Here we have a user trying to update his email, but with an already existing email
# let him login, but display a message because we didn't update his email
Expand Down
23 changes: 14 additions & 9 deletions itou/openid_connect/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,15 @@ def check_valid_kind(self, user, user_data_dict, is_login):

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, 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.
A user is being created or updated from information provided by an identity provider.
A user is globally unique with the combination of SSO provider + sub (e.g. InclusionConnect:username).

If we cannot find the user via provider + username:
- If the email isn't in use, we'll create a new account.
- We will replace a Django account with the same email with the SSO account information.
- We will raise an EmailInUseException if the email is being used by another account of the same SSO
(we do not support email overloading).
- We will return the user without modification if they exist on a different SSO.
"""
user_data_dict = dataclasses.asdict(self)
user_data_dict = {key: value for key, value in user_data_dict.items() if value}
Expand All @@ -131,14 +134,16 @@ def create_or_update_user(self, is_login=False):
created = 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)
created = False
if user.identity_provider not in [IdentityProvider.DJANGO, self.identity_provider]:
if user.identity_provider == self.identity_provider:
raise EmailInUseException(user)
# It is possible to "upgrade" a Django account to an SSO, but not to replace an SSO
if user.identity_provider != IdentityProvider.DJANGO:
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,
Expand Down
11 changes: 9 additions & 2 deletions itou/openid_connect/pe_connect/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,14 @@ def pe_connect_callback(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 ""
msg_who = (
format_html(
" au nom de <strong>{}</strong>",
redacted_name,
)
if redacted_name
else ""
)

return _redirect_to_job_seeker_login_on_error(
format_html(
Expand All @@ -164,7 +171,7 @@ def pe_connect_callback(request):
msg_who,
),
request=request,
extra_tags="modal france_travail_registration_failure",
extra_tags="modal sso_email_conflict_registration_failure",
)

nir = request.session.get(global_constants.ITOU_SESSION_NIR_KEY)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ <h3 class="modal-title text-danger" id="message-modal-{{ forloop.counter }}-labe
<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">
Expand Down
4 changes: 2 additions & 2 deletions itou/templates/utils/modals.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
{% 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" %}
{% if "sso_email_conflict_registration_failure" in message.extra_tags %}
{% include "utils/modal_includes/sso_email_conflict_registration_failure.html" %}
{% endif %}
</div>

Expand Down
12 changes: 11 additions & 1 deletion tests/openid_connect/france_connect/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from itou.openid_connect.constants import OIDC_STATE_CLEANUP
from itou.openid_connect.france_connect import constants
from itou.openid_connect.france_connect.models import FranceConnectState, FranceConnectUserData
from itou.openid_connect.models import InvalidKindException
from itou.openid_connect.models import EmailInUseException, InvalidKindException
from itou.users.enums import IdentityProvider, UserKind
from itou.users.models import User
from tests.users.factories import JobSeekerFactory, PrescriberFactory, UserFactory
Expand Down Expand Up @@ -231,6 +231,16 @@ def test_create_django_user_with_already_existing_fc_email_django(self):
assert user.identity_provider == IdentityProvider.FRANCE_CONNECT
assert user.external_data_source_history != {}

def test_create_django_user_with_already_existing_fc_email(self):
fc_user_data = FranceConnectUserData.from_user_info(FC_USERINFO)
JobSeekerFactory(
username="another_username",
email=fc_user_data.email,
identity_provider=IdentityProvider.FRANCE_CONNECT,
)
with pytest.raises(EmailInUseException):
fc_user_data.create_or_update_user()

def test_create_django_user_with_already_existing_fc_email_other_sso_job_seeker(self):
"""
If there already is an existing job seeker with the email FranceConnect sent us, we do not create it again,
Expand Down
12 changes: 11 additions & 1 deletion tests/openid_connect/inclusion_connect/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
InclusionConnectState,
)
from itou.openid_connect.inclusion_connect.views import InclusionConnectSession
from itou.openid_connect.models import InvalidKindException
from itou.openid_connect.models import EmailInUseException, InvalidKindException
from itou.prescribers.models import PrescriberOrganization
from itou.users import enums as users_enums
from itou.users.enums import IdentityProvider, UserKind
Expand Down Expand Up @@ -261,6 +261,16 @@ def test_get_existing_user_with_same_email_django(self):
assert user.username == OIDC_USERINFO["sub"]
assert user.identity_provider == users_enums.IdentityProvider.INCLUSION_CONNECT

def test_get_existing_user_with_same_email_django_inclusion_connect(self):
ic_user_data = InclusionConnectPrescriberData.from_user_info(OIDC_USERINFO)
PrescriberFactory(
username="another_username",
email=ic_user_data.email,
identity_provider=IdentityProvider.INCLUSION_CONNECT,
)
with pytest.raises(EmailInUseException):
ic_user_data.create_or_update_user()

def test_update_user_from_user_info(self):
user = PrescriberFactory(**dataclasses.asdict(InclusionConnectPrescriberData.from_user_info(OIDC_USERINFO)))
ic_user = InclusionConnectPrescriberData.from_user_info(OIDC_USERINFO)
Expand Down