Project

General

Profile

Development #28853

Plutôt que de demander le mot de passe lors d'une suppression, lancer une réauthentification

Added by Benjamin Dauvergne 11 months ago. Updated 5 months ago.

Status:
Solution proposée
Priority:
Normal
Category:
-
Target version:
-
Start date:
11 Dec 2018
Due date:
% Done:

0%

Patch proposed:
Yes
Planning:
No

Description

Les méthodes d'authentification étant multiple demander le mot de passe pour la suppression d'un compte est contre-productive, à la place il faudrait demander une réauthentification si l'authentification est plus ancien qu'un certain seuil.

Le flowchart:
1. si GET
1.1. si dernière authentification < X minutes:
1.1.1. afficher un formulaire avec un jeton signé permettant le changement du mot de passe
2. si POST
1.2. sinon rediriger vers /login/?next=/accounts/delete/ avec un message indiquant pourquoi une ré-authentification est nécessaire
2.1. si le POST contient un jeton signé valide de moins de 10 minutes : supprimer le compte
2.2. sinon traiter comme un GET (aller en 1)

L'utilisation d'un jeton évite le problème d'un formulaire obtenu juste avant le dépassement du délai (la personne a alors 10 minutes pour compléter le formulaire).

Cas d'usage immédiate: personne connecté via FranceConnect voulant supprimer son compte

0001-views-ask-for-reauthentication-when-deleting-account.patch View (5.64 KB) Benjamin Dauvergne, 16 May 2019 07:03 PM

0001-views-ask-for-reauthentication-when-deleting-account.patch View (5.99 KB) Benjamin Dauvergne, 17 May 2019 09:47 AM

0001-views-ask-for-reauthentication-when-deleting-account.patch View (6.93 KB) Benjamin Dauvergne, 17 May 2019 11:03 AM

0001-views-ask-for-reauthentication-when-deleting-account.patch View (6.92 KB) Benjamin Dauvergne, 17 Jun 2019 07:59 PM

0002-views-ask-for-reauthentication-when-deleting-account.patch View (6.92 KB) Benjamin Dauvergne, 18 Jun 2019 12:05 PM

0001-prefill-username-when-authenticated-28853.patch View (1.21 KB) Benjamin Dauvergne, 18 Jun 2019 12:05 PM


Related issues

Blocks Publik - Development #27081: Intégration cahier des charges FranceConnect Nouveau 08 Oct 2018

History

#1 Updated by Benjamin Dauvergne 11 months ago

#2 Updated by Benjamin Dauvergne 6 months ago

  • Assignee set to Benjamin Dauvergne

#3 Updated by Benjamin Dauvergne 6 months ago

#4 Updated by Benjamin Dauvergne 6 months ago

Correction support Django 1.11 dans les tests.

#5 Updated by Valentin Deniaud 6 months ago

DeleteAccountForm dans forms/profile.py peut être supprimé j'ai l'impression.

#7 Updated by Valentin Deniaud 5 months ago

  • Status changed from Solution proposée to En cours

Il y a un %d orphelin (et il manque la traduction) :

1103                 messages.info(request,
1104                               _('Your last authentication is %d seconds old, '
1105                                 'you need to reauthenticate to delete your account'))

À part ça c'est bon pour moi. En passant, les lignes qui font plus de 100 caractères dans DeleteView gagneraient à être splittées.

Je note aussi que c'est dommage d'avoir perdu le préremplissage du nom d'utilisateur dans le formulaire de login, et que si il y a un moyen simple de le garder il faudrait y songer. Outre l'UX pas terrible, j'ai failli me faire avoir en testant : j'ai créé un nouvel utilisateur pour pouvoir le supprimer, je me suis connecté avec lui et j'ai demandé la suppression au bout d'une minute, et le formulaire de la page de réauthentification s'est affiché prérempli par FF avec mon compte de test habituel ; si je faisais pas gaffe, je supprimais le mauvais compte.

#8 Updated by Frédéric Péters 5 months ago

1104                               _('Your last authentication is %d seconds old, '
1105                                 'you need to reauthenticate to delete your account'))

Plutôt parler aux gens dans des unités pertinentes ?

#9 Updated by Benjamin Dauvergne 5 months ago

Valentin Deniaud a écrit :

Il y a un %d orphelin (et il manque la traduction) :
[...]

À part ça c'est bon pour moi. En passant, les lignes qui font plus de 100 caractères dans DeleteView gagneraient à être splittées.

J'applique une limite de 120.

Je note aussi que c'est dommage d'avoir perdu le préremplissage du nom d'utilisateur dans le formulaire de login, et que si il y a un moyen simple de le garder il faudrait y songer. Outre l'UX pas terrible, j'ai failli me faire avoir en testant : j'ai créé un nouvel utilisateur pour pouvoir le supprimer, je me suis connecté avec lui et j'ai demandé la suppression au bout d'une minute, et le formulaire de la page de réauthentification s'est affiwché prérempli par FF avec mon compte de test habituel ; si je faisais pas gaffe, je supprimais le mauvais compte.

Il faut un POST pour supprimer un compte, si tu reviens de la vue de login la vue de suppression va se ré-afficher parce que tu viens via une redirection et donc un GET. Mais par contre c'est vrai qu'en cas d'erreur d'authent sur la vue de login le message parlant de la vue de suppression va partir; mais c'est un autre ticket et une vieille demande le fait d'avoir des messages persistant sur la page de login.

#10 Updated by Benjamin Dauvergne 5 months ago

Les deux remarques prises en compte :

diff --git a/src/authentic2/views.py b/src/authentic2/views.py
index 3b46dcca..4d54da92 100644
--- a/src/authentic2/views.py
+++ b/src/authentic2/views.py
@@ -1101,7 +1101,7 @@ class DeleteView(FormView):
                 return utils.redirect(request, request.path, params={'token': signer.sign(str(self.request.user.uuid))})
             else:
                 messages.info(request,
-                              _('Your last authentication is %d seconds old, '
+                              _('Your last authentication is too old, '
                                 'you need to reauthenticate to delete your account'))
                 return utils.login_require(request)
         return super(DeleteView, self).dispatch(request, *args, **kwargs)

#11 Updated by Benjamin Dauvergne 5 months ago

#12 Updated by Valentin Deniaud 5 months ago

  • Status changed from Solution proposée to En cours

Je viens de penser que ce patch ne fait pas bon ménage avec le setting A2_LOGIN_REDIRECT_AUTHENTICATED_USERS_TO_HOMEPAGE. Si il est activé (pas le cas par défaut), impossible de supprimer son compte (sans avoir connaissance de la mécanique interne, sinon on se délogge/relogge et on va vite supprimer). C'est cette remarque qui te prive du ack, pas la suite.

Benjamin Dauvergne a écrit :

Je note aussi que c'est dommage d'avoir perdu le préremplissage du nom d'utilisateur dans le formulaire de login, et que si il y a un moyen simple de le garder il faudrait y songer. Outre l'UX pas terrible, j'ai failli me faire avoir en testant : j'ai créé un nouvel utilisateur pour pouvoir le supprimer, je me suis connecté avec lui et j'ai demandé la suppression au bout d'une minute, et le formulaire de la page de réauthentification s'est affiwché prérempli par FF avec mon compte de test habituel ; si je faisais pas gaffe, je supprimais le mauvais compte.

Il faut un POST pour supprimer un compte, si tu reviens de la vue de login la vue de suppression va se ré-afficher parce que tu viens via une redirection et donc un GET. Mais par contre c'est vrai qu'en cas d'erreur d'authent sur la vue de login le message parlant de la vue de suppression va partir; mais c'est un autre ticket et une vieille demande le fait d'avoir des messages persistant sur la page de login.

Ce que je voulais dire c'est que dans le cas d'une authentification par login/mot de passe, le comportement d'avant qui était de simplement redemander le mdp est quand même plus agréable et cohérent que le comportement du patch où il faut réentrer son login (et via des cas limites comme celui que je citais, rien n'empêche de changer de login et donc de compte au moment de cette réauthentification).
Pour améliorer ça il faudrait que la vue de login comprenne que si l'utilisateur est déjà authentifié (ou en rendant ça explicite avec un paramètre GET), il faut ne permettre le login qu'à cet utilisateur (et préremplir + griser ou mieux, cacher comme avant le champ username). Mais c'est sûrement un casse-tête pour rendre ça générique à tous les plugins d'authent possibles.

#13 Updated by Benjamin Dauvergne 5 months ago

Valentin Deniaud a écrit :

Je viens de penser que ce patch ne fait pas bon ménage avec le setting A2_LOGIN_REDIRECT_AUTHENTICATED_USERS_TO_HOMEPAGE. Si il est activé (pas le cas par défaut), impossible de supprimer son compte (sans avoir connaissance de la mécanique interne, sinon on se délogge/relogge et on va vite supprimer). C'est cette remarque qui te prive du ack, pas la suite.

Hmm oui en plus ça impacte le CUT qui est le seul utilisateur de ce setting et ça me fait bien chier; réfléchissement Jean-Pierre...

Benjamin Dauvergne a écrit :

Je note aussi que c'est dommage d'avoir perdu le préremplissage du nom d'utilisateur dans le formulaire de login, et que si il y a un moyen simple de le garder il faudrait y songer. Outre l'UX pas terrible, j'ai failli me faire avoir en testant : j'ai créé un nouvel utilisateur pour pouvoir le supprimer, je me suis connecté avec lui et j'ai demandé la suppression au bout d'une minute, et le formulaire de la page de réauthentification s'est affiwché prérempli par FF avec mon compte de test habituel ; si je faisais pas gaffe, je supprimais le mauvais compte.

Il faut un POST pour supprimer un compte, si tu reviens de la vue de login la vue de suppression va se ré-afficher parce que tu viens via une redirection et donc un GET. Mais par contre c'est vrai qu'en cas d'erreur d'authent sur la vue de login le message parlant de la vue de suppression va partir; mais c'est un autre ticket et une vieille demande le fait d'avoir des messages persistant sur la page de login.

Ce que je voulais dire c'est que dans le cas d'une authentification par login/mot de passe, le comportement d'avant qui était de simplement redemander le mdp est quand même plus agréable et cohérent que le comportement du patch où il faut réentrer son login (et via des cas limites comme celui que je citais, rien n'empêche de changer de login et donc de compte au moment de cette réauthentification).

Ok je comprends, mais je préférerai traiter ça dans un autre ticket :
1. parce que peu de gens suppriment leur compte, donc cette petite gêne n'est pas exagérée
2. ça rejoint un besoin plus large de login_hint1 (jargon OIDC) où il est parfois possible de passer une indication sur l'identité de l'utilisateur; exemple en OIDC on peut mettre login_hint=<email> ou id_token_hint=<id_token> pour aider l'IdP à trouver l'utilisateur (en pré-remplissant le champ username, en redirigeant automatiquement vers un autre IdP s'il sait que c'est un utilisateur externe, en sélectionnant le bon compte si c'est un IdP gérant de multiple session en parallèle, voir google).

Mais dans le cas où l'utilisateur est déjà connecté je vais voir pour améliorer les choses.

1 https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest

Pour améliorer ça il faudrait que la vue de login comprenne que si l'utilisateur est déjà authentifié (ou en rendant ça explicite avec un paramètre GET), il faut ne permettre le login qu'à cet utilisateur (et préremplir + griser ou mieux, cacher comme avant le champ username). Mais c'est sûrement un casse-tête pour rendre ça générique à tous les plugins d'authent possibles.

Je vais déjà pré-remplir et passer le focus sur le deuxième champ.

#14 Updated by Benjamin Dauvergne 5 months ago

Déjà le prefill du username.

Also available in: Atom PDF