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

Merged
pmarillonnet merged 5 commits from wip/69890-password-lost-sms-recovery into main 2023-05-04 15:06:02 +02:00
Owner
No description provided.
pmarillonnet force-pushed wip/69890-password-lost-sms-recovery from 0aa2630f3c to 6d2fd3ab58 2023-04-03 14:49:49 +02:00 Compare
pmarillonnet changed title from 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) 2023-04-03 14:51:12 +02:00
Owner

Je ne comprends toujours pas. Voilà comment je vois les choses :

   GET /password/reset/
      
      <form>
         <lable>Numéro de téléphone</label><input name="telephone">
      </form>

La personne rentre son téléphone :

   POST /password/reset/
   telephone=0033699999999

Là on lance dans tous les cas:

authentic2.multitenant.spooler.run(create_token_and_send_sms_if_account_exists, telephone='0033699999999')

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 :

 <form>
     <p>Si un compte existe pour le numéro +33 06.99.99.99.99 vous allez recevoir un code sous peu.</p>
     <input type="hidden" name="telephone" value="0033699999999">
     <label>Code</label><input name="code">
     <button>Vérifier le code</button> <a href="/password/reset/" class"button">Choisir un autre numéro...</a>
  </form>

Là on vérifie si un token existe pour le code et le numéro :

   POST /password/reset/
   code=1234
   
   <form>
         <input type="hidden" name="telephone" value="0033699999999">
         <input type="hidden" name="code" value="1234">
         <label>Nouveau mot de passe</label>
         <input name="password1">
         <label>Nouveau mot de passe (confirmation)</label>
         <input name="password2">
   </form>

Si tout ça valide, on change le mot de passe et on log la personne (là c'est comme avant).

Je ne comprends toujours pas. Voilà comment je vois les choses : ``` GET /password/reset/ <form> <lable>Numéro de téléphone</label><input name="telephone"> </form> ``` La personne rentre son téléphone : ``` POST /password/reset/ telephone=0033699999999 ``` Là on lance dans tous les cas: authentic2.multitenant.spooler.run(create_token_and_send_sms_if_account_exists, telephone='0033699999999') 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 : <form> <p>Si un compte existe pour le numéro +33 06.99.99.99.99 vous allez recevoir un code sous peu.</p> <input type="hidden" name="telephone" value="0033699999999"> <label>Code</label><input name="code"> <button>Vérifier le code</button> <a href="/password/reset/" class"button">Choisir un autre numéro...</a> </form> Là on vérifie si un token existe pour le code et le numéro : ``` POST /password/reset/ code=1234 <form> <input type="hidden" name="telephone" value="0033699999999"> <input type="hidden" name="code" value="1234"> <label>Nouveau mot de passe</label> <input name="password1"> <label>Nouveau mot de passe (confirmation)</label> <input name="password2"> </form> ``` Si tout ça valide, on change le mot de passe et on log la personne (là c'est comme avant).
pmarillonnet force-pushed wip/69890-password-lost-sms-recovery from 6d2fd3ab58 to 51c1798ddd 2023-04-20 09:28:03 +02:00 Compare
Author
Owner

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

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.

> 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). 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.
Author
Owner

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

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

> > 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). > > 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.)
Owner

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 ?

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 ?
Owner

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.

> 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.
Author
Owner

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 ?

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

class InputSMSCodeView(cbv.ValidateCSRFMixin, FormView):
    # […]
    def dispatch(self, request, *args, **kwargs):
        token = kwargs.get('token')
        try:
            self.code = models.SMSCode.objects.get(url_token=token)
        except models.SMSCode.DoesNotExist:
            return HttpResponseBadRequest(_('Invalid request'))

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 :

> 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 ? 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 ``` class InputSMSCodeView(cbv.ValidateCSRFMixin, FormView): # […] def dispatch(self, request, *args, **kwargs): token = kwargs.get('token') try: self.code = models.SMSCode.objects.get(url_token=token) except models.SMSCode.DoesNotExist: return HttpResponseBadRequest(_('Invalid request')) ``` 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 :
Author
Owner

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.

> 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.
Author
Owner

Techniquement parlant, cela se traduit en :

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

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

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

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

Techniquement parlant, cela se traduit en : > · 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. -> 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. > · 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. -> 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.
Owner

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

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()@.

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

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

> · 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. 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()@. > · 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. 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).
bdauvergne approved these changes 2023-04-24 18:07:57 +02:00
bdauvergne left a comment
Owner

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.

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

Pas besoin de retourner si la requête est légitime ou pas.

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()
Owner

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.

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,
Owner

Si c'est bien conçu, on ne saura jamais si les SMS partent, on ne peut donc pas le signaler aux gens.

Si c'est bien conçu, on ne saura jamais si les SMS partent, on ne peut donc pas le signaler aux gens.
Author
Owner

Là oui c’est juste quand vraiment rien ne marche et que le requests.post au connecteur d’envoi de SMS lève une RequestException, 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.

Là oui c’est juste quand vraiment rien ne marche et que le `requests.post` au connecteur d’envoi de SMS lève une `RequestException`, 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):
Owner

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.

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

C'est pas essentiel à la relecture, j'ouvrirai des tickets.

C'est pas essentiel à la relecture, j'ouvrirai des tickets.
Author
Owner

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.

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.
bdauvergne requested changes 2023-04-24 18:08:28 +02:00
bdauvergne left a comment
Owner

Mauvais bouton.

Mauvais bouton.
pmarillonnet force-pushed wip/69890-password-lost-sms-recovery from 51c1798ddd to 40fee82cbe 2023-05-03 10:06:28 +02:00 Compare
pmarillonnet force-pushed wip/69890-password-lost-sms-recovery from 40fee82cbe to d8bee503ef 2023-05-03 11:00:52 +02:00 Compare
pmarillonnet force-pushed wip/69890-password-lost-sms-recovery from d8bee503ef to 74001b0c73 2023-05-03 11:19:53 +02:00 Compare
Author
Owner

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.

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 flag sent 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.

> 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. 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 flag `sent` 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.
pmarillonnet requested review from bdauvergne 2023-05-03 11:53:43 +02:00
bdauvergne approved these changes 2023-05-04 14:31:04 +02:00
bdauvergne left a comment
Owner

ok

ok
@ -815,3 +815,3 @@
CODE_DURATION = 120
KIND_REGISTRATION = 'registration'
KIND_PASSWORD_LOST = 'password-lost'
KIND_PASSWORD_LOST = 'pw-reset'
Owner

Tant qu'à changer met 'password-reset'.

Tant qu'à changer met 'password-reset'.
Author
Owner

D’ac, je fais la modif et je merge, merci.

D’ac, je fais la modif et je merge, merci.
pmarillonnet force-pushed wip/69890-password-lost-sms-recovery from 74001b0c73 to ba9550511e 2023-05-04 14:54:46 +02:00 Compare
pmarillonnet merged commit ba9550511e into main 2023-05-04 15:06:02 +02:00
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/authentic#31
No description provided.