custom_user : récupération du compte par envoi d’un code au numéro vérifié lorsque le mot de passe a été oublié par l’usager (#69890) #31
No reviewers
Labels
No Label
No Milestone
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: entrouvert/authentic#31
Loading…
Reference in New Issue
No description provided.
Delete Branch "wip/69890-password-lost-sms-recovery"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
0aa2630f3c
to6d2fd3ab58
WIP: custom_user : récupération du compte par envoi d’un code au numéro vérifié lorsque le mot de passe a été oublié par l’usager (#69890)to custom_user : récupération du compte par envoi d’un code au numéro vérifié lorsque le mot de passe a été oublié par l’usager (#69890)Je ne comprends toujours pas. Voilà comment je vois les choses :
La personne rentre son téléphone :
Là on lance dans tous les cas:
Envoi via spooler pour ne pas qu'on puisse détecter si oui ou non un compte existe via le temps que prend la requête. Suite à ce POST on retourne ce formulaire :
Là on vérifie si un token existe pour le code et le numéro :
Si tout ça valide, on change le mot de passe et on log la personne (là c'est comme avant).
6d2fd3ab58
to51c1798ddd
C’est précisément ce qui se passe dans cette PR oui, à deux exceptions près :
· dans cette version on n’utilise pas le spooler a2 multitenant, simplement une fonction utilitaire dans
authentic2.utils.sms
, comme pour l’enregistrement. Peut-être qu’on peut déjà passer cela comme ça, et voir dans une PR suivante pour l’utilisation du spooler ?· pour l’instant la page de saisie du code reçue est une page neutre générique qui indique “Saisissez le code reçu par SMS”, c’est le même gabarit qui est utilisé pour la saisie du code d’inscription et la saisie du code de récupération. Je me disais qu’on pourrait varier les pages et choisir de jolis messages dans un PR suivante aussi, que ce qui compte ici est la mécanique de récupération du mot de passe.
Voilà, je pense que la PR reste bonne pour relecture au regard de ce qui a été discuté ici.
(En particulier ce que tu mentionnes ici, à savoir utiliser le spooler pour qu’un attaquant ne puisse pas deviner si le compte existe ou non en fonction du temps d’exécution de la requête, je pense vraiment que ça peut venir dans un second temps, indépendamment de cette PR.)
L'important dans ma description c'est qu'à aucun moment je n'ai besoin d'un code "fake" (l'histoire du spooler est accessoire). Est-ce qu'on pourrait enfin savoir à quoi cela sert ?
Précisions: c'est accessoire dans le sens où oui on peut faire sans ici, ou utiliser un thread au pire pour simuler le coté non bloquant.
Alors on n’envoie pas de code, mais en effet on crée un objet en base qui stocke l'uuid opaque de la demande de ré-initialisation, et ceci afin de retenir le fait qu’une demande a été faite, même si celle-ci est pour un numéro inexistant.
En plus d’être plus simple techniquement, cela permet d’éviter que la vue qui présente le formulaire de saisie du code (supposé ou bien réellement reçu par l’usager) accepte en paramètre d’URL tout ce qui ressemble à peu près à un UUID. Au contraire la vue est capable de reconnaître que la demande a bien eu lieu (quand bien même elle correspond à un numéro inconnu d’authentic). Typiquement dans le code c’est
Bien sûr on pourrait faire sans créer d’objet du tout en base lorsque le numéro est inconnu, mais alors on ne serait plus en mesure de discerner les deux cas suivants :
(Pardon message tronqué)
Les deux cas, fonctionnellement parlant :
· le formulaire de saisie du code est présenté à l’usager pour une demande de ré-initialisation de mot de passe qui n’a jamais eu lieu, ou bien qui est périmée.
· le formulaire de saisie du code est présenté à l’usager pour une demande de ré-initialisation effectivement soumise mais pour un numéro inconnu d’authentic.
Ne pas discerner ces deux cas ce serait à mon avis une erreur technique et fonctionnelle.
Techniquement parlant, cela se traduit en :
-> Pas d’objet SMSCode trouvé en base pour cette URL opaque. L’URL a été forgée ou est trop ancienne, et on peut raisonnablement claquer une erreur.
-> Un objet effectivement trouvé en base, mais avec avec fake=True. Le numéro est inconnu d’a2 mais l’usager n’a pas à le savoir, on lui présente le formulaire comme s’il y avait eu envoi d’un SMS.
Ce cas n'est pas possible, pour l'email on envoie des liens par mail, qui traînent ensuite dans la boite mail des gens qui peuvent revenir dessus et cliquer, effectivement on essaie dans ce cas de leur dire, "Votre lien est périmé". Mais ici on enchaîne directement sur le formulaire pour rentrer le code, le formulaire ne peut pas être périmé il n'a d'ailleurs pas besoin d'un uuid ou aucune référence au code envoyé, juste le numéro de téléphone et il doit juste recherche un jeton @SmsCode.objects.filter(phone=code, code=code).exists()@.
Je crois aussi que ça n'a pas d'importance, c'est un tunnel, on a pas besoin de garder toute cet état en base, ça s'enchaîne.
Évacuons mon histoire de time-oracle, ok on envoie le SMS dans le thread de la requête, mais on ensuite il faut juste rediriger sur une vue avec le numéro de téléphone en paramètre et c'est tout (ça peut être la même vue pour enregistrement et reset, pas de souci, il suffit de vérifier le type dans l'objet code si on trouve le code).
Je pense que c'était déjà trop compliqué pour l'enregistrement, l'uuid de smscode n'est pas nécessaire en fait et les distinctions entre code périmé/pas envoyé non plus, soit on trouve soit on ne trouve pas, au bout de 2/3 essais ça propose de renvoyer un code. Je ne connais pas de site qui en disent plus que ça.
@ -143,0 +202,4 @@
elif phone:
# TODO hook(?)
return (None, sms_sent)
return (None, False)
Pas besoin de retourner si la requête est légitime ou pas.
@ -935,0 +969,4 @@
)
return self.form_invalid(form)
self.code, legitimate_request = form.save()
Ne rien retourner comme pour le mail, simplement au lieu d'un redirect vers self.success_url, on redirige vers la vue pour rentrer le code directement ici en testant si form.cleaned_data['phone'] existe et on passe phone en paramètre.
@ -935,0 +973,4 @@
if not self.code:
if legitimate_request:
messages.error(
self.request,
Si c'est bien conçu, on ne saura jamais si les SMS partent, on ne peut donc pas le signaler aux gens.
Là oui c’est juste quand vraiment rien ne marche et que le
requests.post
au connecteur d’envoi de SMS lève uneRequestException
, c’est juste dans ce cas très précis qu’on peut d’ores et déjà dire à l’usager que ce n’est pas la peine qu’il surveille l’arrivée du SMS sur son tél. Dans le cas général on ne saura pas si le SMS est parti ou non.@ -1248,3 +1302,3 @@
title = _('Account activation')
title = _('SMS code validation')
def dispatch(self, request, *args, **kwargs):
Même si l'espace de recherche est super grand (et je pense que c'est même trop, enfin on verra) j'aurai mis un ratelimit au moins pour voir qu'une attaque est en cours de ce coté ci et dissuader les ambitieux. Aussi dans un prochain temps il faudrait prévoir sur une erreur de proposer de renvoyer un code.
C'est pas essentiel à la relecture, j'ouvrirai des tickets.
Oui je me disais que le ratelimiting de ce coté là pourrait arriver dans un second temps, via un autre ticket. Je vais le créer maintenant pour ne pas oublier.
Mauvais bouton.
51c1798ddd
to40fee82cbe
40fee82cbe
tod8bee503ef
d8bee503ef
to74001b0c73
Je te propose à la relecture une nouvelle version qui :
· laisse tomber cette histoire de
(code, legitimate_request) = form.save()
. Juste la sauvegarde du formulaire renvoie l’objet code sans chercher à savoir si la requête est légitime ou non,· simplifie les conditions de présentation du formulaire de saisie du SMS (
InputSMSCodeView.dispatch
nettement simplifiée). Notamment on abandonne le flagsent
sur l’objet SMSCode.En revanche j’ai préféré conserver l’UUID de SMSCode, ça me va de reprendre la même logique que pour la création de compte, et puis ça me gêne que juste
self.cleaned_data['phone']
soit passé en paramètre du formulaire de saisie du code, l’URL duquel devient alors entièrement devinable (pour un attaquant qui sait que l’usager vient de soumettre une demande de réinitialisation de mot de passe et qui peut alors tenter sa chance de son coté dans le formulaire de saisie du code). Je préfère que ça reste des URLs opaques.Pour le ratelimiting sur la saisie du code j’ai créé #77245, je pense comme toi que ça peut arriver dans un second temps.
ok
@ -815,3 +815,3 @@
CODE_DURATION = 120
KIND_REGISTRATION = 'registration'
KIND_PASSWORD_LOST = 'password-lost'
KIND_PASSWORD_LOST = 'pw-reset'
Tant qu'à changer met 'password-reset'.
D’ac, je fais la modif et je merge, merci.
74001b0c73
toba9550511e