Produits Entr'ouvert » Authentic 2

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

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 ?

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.

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)

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_hint¹ (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.

¹ 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.

Première remarque, une ligne qui attise ma curiosité (je continue à relire le reste en attendant) :

    # Create blocks
    for authenticator in authenticators:
        if methods and not (authenticator.id in methods or set(authenticator.how) & set(methods)):
            continue

———

Edit : autre remarque sur une autre ligne qui m’interpelle, dans la nouvelle méthode AccountDeleteView.has_recent_authentication :

age = time.time() - utils_misc.last_authentication_event(request=self.request)['when']

si pour une raison ou une autre on ne retrouve pas d’événement pour la requête, genre des objets AuthenticationEvent manquants, alors last_authentication_event renvoie @None et cette ligne va tracer.

Paul Marillonnet a écrit :

Première remarque, une ligne qui attise ma curiosité (je continue à relire le reste en attendant) :
[...]
Pourquoi on en arrive à tester soit l’id de l’authentificateur ou bien son attribut how ? C’est l’existant sur GLC qui justifie de devoir tester les des deux à la fois ?

Petit historique d'authentic et du patch: pour des raisons qui ont à voir avec SAML qui fait la différence (différence dont on se fout aujourd'hui) le mode "mot de passe" a deux valeurs pour "how" : password et password-on-https. Pour des raisons de bêtise ou de ticket que j'ai accepté sans réfléchir le module franceconnect a pour identifiant "fc" mais comme méthode "france-connect", pour simplifier j'ai fait id in method pendant l'écriture, puis ça marchait pas pour franceconnect donc j'ai ajouté method in self.hows puis après je me suis dit qu'on pourrait vouloir plusieurs méthode et c'est devenu ça.

Finalement ça ne marche pour authentic2_auth_saml où on peut condiérer que self.hows == [self.id] (y a pas de self.hows déclaré, vu que c'est toujours saml). Si tu trouves ça con, je rajoute un class SAMLAuthenticator\hows = ['saml'] et je vire la condition.

PS:

Je ne vois nulle part dans le code le cas où l’identifiant de l’authentificateur aurait été passé dans methods.

Oui c'est dans les appels à misc_utils.login(...) la chaîne est généralement écrite directement.

Pour des raisons de bêtise ou de ticket que j'ai accepté sans réfléchir le module franceconnect a pour identifiant "fc" mais comme méthode "france-connect",

Si jamais quelqu'un cherche ça arrive dans #21908 qui ajoute

+        data['authentication_method'] = 'france-connect'

dans le module qui a "fc" comme id.

Benjamin Dauvergne a écrit :

Petit historique d'authentic et du patch: pour des raisons qui ont à voir avec SAML qui fait la différence (différence dont on se fout aujourd'hui) le mode "mot de passe" a deux valeurs pour "how" : password et password-on-https. Pour des raisons de bêtise ou de ticket que j'ai accepté sans réfléchir le module franceconnect a pour identifiant "fc" mais comme méthode "france-connect", pour simplifier j'ai fait id in method pendant l'écriture, puis ça marchait pas pour franceconnect donc j'ai ajouté method in self.hows puis après je me suis dit qu'on pourrait vouloir plusieurs méthode et c'est devenu ça.

Merci pour l’historique et merci à Fred d’avoir identifié le ticket en question.

Finalement ça ne marche pour authentic2_auth_saml où on peut condiérer que self.hows == [self.id] (y a pas de self.hows déclaré, vu que c'est toujours saml). Si tu trouves ça con, je rajoute un class SAMLAuthenticator\hows = ['saml'] et je vire la condition.

Oui je pense que ce serait mieux en virant cette condition.

Automatic expiration