Connexion: Utilisation du modèle EmailAddress et refonte du parcours de modification d'adresse mail [GEN-2216] #5088

calummackervoy · 2024-11-12T16:01:13Z

🤔 Pourquoi ?

Quand un utilisateur modifie son email, l'accès à son compte est bloqué jusqu'à ce qu'il soit vérifié. S'il s'est trompé, il n'aura pas accès à son compte jusqu'à il peut accèder l'adresse mail ou il est débloqué par le support. Toutefois, il ne recevra cet e-mail avant qu'il essaie de se connecter avec le nouvelle adresse.

🍰 Comment ?

Ce PR propose une refonte du parcours de modification plus clair, et que la notification soit envoyée immédiatement, et que l'utilisateur soit rédirigé vers la page d'instruction.

Un utilisateur peut démander une seconde adresse mail, et le moment que il le vérifie son user.email sera remplacé. Si l'utilisateur se connecte avec son ancien e-mail il peut, mais ça va annuler sa réquête à changement d'adresse mail. S'il démande un 3ème la réquête sera remplacée et l'adresse mail en cours de vérification annulée/relâchée.

Tandis qu'un e-mail est en cours de vérification, il est réservé et un tiers ne peut pas inscrire sur notre plate-forme en l'utilisant.

Impliqué par ces changements est que le modèle EmailAddress doit être utilisé lorsque que l'unicité d'adresse mail est importante, car un EmailAddress en cours de vérification est quand même réservé au utilisateur qui l'a démandé. Il inclut donc des changements au User.save pour gerer la création / modification des adresses mails, et une migration temporaire pour remplir toutes les adresses mails manquante sur le base de données du prod.

🚨 À vérifier

Mettre à jour le CHANGELOG_breaking_changes.md ?

🏝️ Comment tester

Se connecter avec un compte existant
Modifier votre adresse mail : http://localhost:8000/dashboard/edit_user_email
Vous serez déconnecté et redirigé vers http://localhost:8000/accounts/confirm-email/
Vous recevrez un email de vérification

notion-workspace · 2024-11-12T16:01:17Z

☠️ ETQU lorsque je change d’adresse email, je dois confirmer cette nouvelle adresse

github-actions · 2024-11-12T16:15:18Z

🥁 La recette jetable est prête ! 👉 Je veux tester cette PR !

itou/www/dashboard/views.py

calummackervoy · 2024-11-25T06:53:03Z

itou/templates/account/email/email_confirmation_signup_message.txt

+{% autoescape off %}
+Nous avons bien enregistré votre demande d'inscription et vous remercions de votre confiance.
+
+Afin de finaliser votre inscription, cliquez sur le lien suivant :
+
+{{ activate_url }}
+
+Si vous n'êtes pas à l'origine de cette demande, merci de ne pas prendre en compte ce message.
+{% include "layout/base_email_signature.txt" %}
+{% endautoescape %}


Fonctionnalité de django-allauth, ce gabarit sera utilisé lorsque signup=True (docs)

calummackervoy · 2024-11-26T14:00:07Z

itou/users/migrations/0015_ensure_emailaddress_model_use.py

+    EmailAddress.objects.bulk_create(
+        EmailAddress(user=x.user, email=x.email, primary=True, verified=False) for x in users_missing_addresses
+    )


J'aurais aimé éviter verified=False dans le mesure de possible. Avec notre système du courant la logique verified=(x.last_login_date > x.modified_at) serait fiable, mais on ne stocke pas un champ modified_at

github-actions · 2024-11-26T14:06:19Z

🥁 La recette jetable est prête ! 👉 Je veux tester cette PR !

Previously the user would need to try to login before the notification was sent Fix requesting email modification does not lock users out of their account Better conflict handling in EditUserEmailForm Ensure EmailAddress consistency when user email changes

Use EmailAddress model in user query email_already_exists Override create_user to prevent EmailAddress conflicts Fix language in error of create_user May be shown to users Creation of candidate by third party considers EmailAddress (JobSeekerExistsForm)

Many users on production have User.email set with no corresponding email address model Fix migration should assume emails are not verified Fix migration typo

Emails are case insensitive (RFC 5321 Part 2.4)

xavfernandez

J'ai un peu l'impression qu'on profite de l'existence du modèle EmailAddress de django-allauth pour implémenter notre changement d'adresse email.
Je me demande s'il ne faudrait pas plutôt le faire avec notre modèle à nous plutôt que de donner l'impression d'utiliser (en la détournant fortement) la fonctionnalité de django-allauth ?

xavfernandez · 2024-12-09T12:37:51Z

itou/openid_connect/inclusion_connect/views.py

@@ -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__iexact=email).first()


Suggested change

email_address = EmailAddress.objects.filter(email__iexact=email).first()

email_address = EmailAddress.objects.filter(email__iexact=email).select_related("user").first()

Et a priori à chaque fois qu'on utilise EmailAddress pour récupérer un User.

xavfernandez · 2024-12-09T12:45:01Z

itou/users/migrations/0015_ensure_emailaddress_model_use.py

+        .values("id", "email")
+    )
+
+    EmailAddress.objects.bulk_create(


Il y en a beaucoup ? Ça vaudrait ptet le coup de faire un migration atomic = False.

xavfernandez · 2024-12-09T12:45:47Z

itou/users/migrations/0015_ensure_emailaddress_model_use.py

+        migrations.RunPython(
+            create_email_addresses_for_users,
+            reverse_code=migrations.RunPython.noop,
+        ),


Tu peux rajouter elidable=True si elle peut être squashée.

Bien vu merci !

xavfernandez · 2024-12-09T12:49:29Z

itou/users/models.py

+        queryset = cls.objects.filter(
+            id__in=EmailAddress.objects.filter(email__iexact=email).values_list("user_id", flat=True)
+        )
        if exclude_pk:
            queryset = queryset.exclude(pk=exclude_pk)


Suggested change

queryset = cls.objects.filter(

id__in=EmailAddress.objects.filter(email__iexact=email).values_list("user_id", flat=True)

)

if exclude_pk:

queryset = queryset.exclude(pk=exclude_pk)

queryset = EmailAddress.objects.filter(email__iexact=email)

if exclude_pk:

queryset = queryset.exclude(user_id=exclude_pk)

calummackervoy · 2024-12-09T15:20:14Z

@xavfernandez

Je me demande s'il ne faudrait pas plutôt le faire avec notre modèle à nous plutôt que de donner l'impression d'utiliser (en la détournant fortement) la fonctionnalité de django-allauth ?

Je suis pour l'utilisation du modèle de django-allauth je pense, car ces adresses sont bien imbriquées dans les concernes d'allauth (l'envoie des e-emails, leur verification, leur association au compte utilisateur) - et on gère les EmailAddress ici avec le fonctionnement d'allauth bien en tête.

Qu'est ce que tu penses ?

tonial · 2024-12-10T08:04:43Z

config/urls.py

+    # We want to control how EmailAddress is managed during this view
+    re_path(
+        r"^accounts/confirm-email/(?P<key>[-:\w]+)/$",
+        signup_views.ItouConfirmEmailView.as_view(),


Pourquoi ne pas garder celle de django-allauth ?

tonial · 2024-12-10T08:16:57Z

Je suis pas forcément très convaincu de la direction de la PR :
On utilise assez peu de fonctionnalités de la lib (connexion avec l'email au lieu du username, et confirmation de l'email)
À côté de ça on a les connexions SSO (ProConnect, FranceConnect et FTConnect) qui sont gérées séparément et que tu es obligé de rebrancher sur la gestion des EmailAdresse de django-allauth.
On se rend compte qu'on a des vues qui sont présentes sans qu'on s'en rendre compte (mais un hackeur pourrait le savoir
Je pense qu'on ferait mieux de retirer django-allauth

calummackervoy added 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC no-changelog Ne doit pas figurer dans le journal des changements. labels Nov 12, 2024

calummackervoy requested review from francoisfreitag, celine-m-s and tonial November 12, 2024 16:01

calummackervoy self-assigned this Nov 12, 2024

calummackervoy force-pushed the calum/modification-adresse-mail branch from 8ba6071 to b9f6941 Compare November 12, 2024 16:04

calummackervoy added 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC and removed 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC labels Nov 12, 2024

tonial requested changes Nov 13, 2024

View reviewed changes

itou/www/dashboard/views.py Outdated Show resolved Hide resolved

calummackervoy added the bug label Nov 19, 2024

calummackervoy marked this pull request as draft November 20, 2024 16:30

calummackervoy force-pushed the calum/modification-adresse-mail branch from b9f6941 to 9092579 Compare November 20, 2024 16:56

calummackervoy commented Nov 25, 2024

View reviewed changes

calummackervoy force-pushed the calum/modification-adresse-mail branch from 9092579 to 6a40156 Compare November 26, 2024 11:58

calummackervoy removed the 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC label Nov 26, 2024

calummackervoy force-pushed the calum/modification-adresse-mail branch from 6a40156 to 8827113 Compare November 26, 2024 13:51

calummackervoy marked this pull request as ready for review November 26, 2024 13:52

calummackervoy requested review from tonial and xavfernandez November 26, 2024 13:52

calummackervoy commented Nov 26, 2024

View reviewed changes

calummackervoy added the 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC label Nov 26, 2024

calummackervoy changed the title ~~Connexion: Envoie de confirmation pendant la modification d'adresse mail [GEN-2216]~~ Connexion: Utilisation du modèle EmailAddress et refonte du parcours de modification d'adresse mail [GEN-2216] Nov 26, 2024

calummackervoy force-pushed the calum/modification-adresse-mail branch from 8827113 to 10f0e77 Compare November 26, 2024 15:43

calummackervoy added 3 commits November 26, 2024 17:15

Add a migration to ensure EmailAddress model populated

4bd1bb3

Many users on production have User.email set with no corresponding email address model Fix migration should assume emails are not verified Fix migration typo

Fix email case sensitivity in account creation/modification

2602f6d

Emails are case insensitive (RFC 5321 Part 2.4)

calummackervoy force-pushed the calum/modification-adresse-mail branch from 10f0e77 to 2602f6d Compare November 26, 2024 16:15

xavfernandez reviewed Dec 9, 2024

View reviewed changes

tonial reviewed Dec 10, 2024

View reviewed changes

calummackervoy removed the 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC label Dec 12, 2024

calummackervoy marked this pull request as draft December 12, 2024 18:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Connexion: Utilisation du modèle EmailAddress et refonte du parcours de modification d'adresse mail [GEN-2216] #5088

Connexion: Utilisation du modèle EmailAddress et refonte du parcours de modification d'adresse mail [GEN-2216] #5088

calummackervoy commented Nov 12, 2024 •

edited

Loading

notion-workspace bot commented Nov 12, 2024

github-actions bot commented Nov 12, 2024

calummackervoy Nov 25, 2024 •

edited

Loading

calummackervoy Nov 26, 2024

github-actions bot commented Nov 26, 2024

xavfernandez left a comment

xavfernandez Dec 9, 2024

xavfernandez Dec 9, 2024

xavfernandez Dec 9, 2024

calummackervoy Dec 9, 2024

xavfernandez Dec 9, 2024

calummackervoy commented Dec 9, 2024

tonial Dec 10, 2024

tonial commented Dec 10, 2024

	email_address = EmailAddress.objects.filter(email__iexact=email).first()
	email_address = EmailAddress.objects.filter(email__iexact=email).select_related("user").first()

Connexion: Utilisation du modèle EmailAddress et refonte du parcours de modification d'adresse mail [GEN-2216] #5088

Are you sure you want to change the base?

Connexion: Utilisation du modèle EmailAddress et refonte du parcours de modification d'adresse mail [GEN-2216] #5088

Conversation

calummackervoy commented Nov 12, 2024 • edited Loading

🤔 Pourquoi ?

🍰 Comment ?

🚨 À vérifier

🏝️ Comment tester

notion-workspace bot commented Nov 12, 2024

github-actions bot commented Nov 12, 2024

calummackervoy Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

calummackervoy Nov 26, 2024

Choose a reason for hiding this comment

github-actions bot commented Nov 26, 2024

xavfernandez left a comment

Choose a reason for hiding this comment

xavfernandez Dec 9, 2024

Choose a reason for hiding this comment

xavfernandez Dec 9, 2024

Choose a reason for hiding this comment

xavfernandez Dec 9, 2024

Choose a reason for hiding this comment

calummackervoy Dec 9, 2024

Choose a reason for hiding this comment

xavfernandez Dec 9, 2024

Choose a reason for hiding this comment

calummackervoy commented Dec 9, 2024

tonial Dec 10, 2024

Choose a reason for hiding this comment

tonial commented Dec 10, 2024

calummackervoy commented Nov 12, 2024 •

edited

Loading

calummackervoy Nov 25, 2024 •

edited

Loading