Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ne demande pas de mot de passe aux utilisateurs qui n'en ont pas #6420

Merged
merged 1 commit into from
Dec 1, 2022

Conversation

Situphen
Copy link
Member

@Situphen Situphen commented Nov 8, 2022

Dans les formulaires sensibles, on continue d'exiger le mot de passe actuel pour les utilisateurs qui en ont un (ceux qui se sont inscrits de manière classique), mais on ne le demande pas aux utilisateurs qui n'en ont pas (ceux qui se connectent via leur réseau social).

Cette vérification est faite via la fonction User.has_usable_password() fournie par Django (et pas avec le champ User.password).

Fixes #6419

QA : Étant donné que la connexion avec les réseaux sociaux ne fonctionnent pas actuellement en local, il faudra vérifier le bon fonctionnement sur la bêta. Pour la même raison, il me parait difficile d'ajouter des tests de non régression là-dessus.

Sur les pages de désinscription, changement de pseudo et courriel ainsi que changement de mot de passe :

  • avec un compte inscrit de manière classique, vérifier que le mot de passe actuel est bien demandé ;
  • avec un compte inscrit via les réseaux sociaux, vérifier que le mot de passe actuel n'est pas demandé ;
  • avec un compte inscrit via les réseaux sociaux,
    • créer un mot de passe via la page changement de mot de passe,
    • puis se reconnecter (c'est déjà le comportement actuel),
    • puis vérifier que le mot de passe actuel est bien demandé.

@Situphen Situphen added C-Back Concerne le back-end Django Bloquant Ticket qui doit être traité avant la prochaine mise à jour labels Nov 8, 2022
@AmauryCarrade AmauryCarrade enabled auto-merge (rebase) November 8, 2022 16:23
@coveralls
Copy link

coveralls commented Nov 8, 2022

Coverage Status

Coverage decreased (-0.003%) to 88.299% when pulling 874a0f1 on Situphen:mdp into 6e9ad77 on zestedesavoir:dev.

Copy link
Contributor

@Arnaud-D Arnaud-D left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je crois que ta solution ne marche pas en l'état. Le champ password est obligatoire, et ne pas l'afficher ne le retire pas de la validation que fait Django. Il faudrait probablement changer la validation du champ (ou sa définition, ou la manière dont le formulaire est instancié, ...), pour qu'il ne soit plus obligatoire.

Autrement, il me semble que tu ajoutes un élément None dans la liste de champs filés à crispy, vu que la fonction insert_... ne retourne rien dans certains cas. Faudrait voir comment crispy se comporte avec ça, mais dans l'absolu, c'est pas très propre.

@Situphen
Copy link
Member Author

Je crois que ta solution ne marche pas en l'état. Le champ password est obligatoire, et ne pas l'afficher ne le retire pas de la validation que fait Django. Il faudrait probablement changer la validation du champ (ou sa définition, ou la manière dont le formulaire est instancié, ...), pour qu'il ne soit plus obligatoire.

Effectivement tu as raison ça ne fonctionnait pas en l'état et il m'a fallu supprimer le champ password du dictionnaire self.fields pour que cela fonctionne.

Autrement, il me semble que tu ajoutes un élément None dans la liste de champs filés à crispy, vu que la fonction insert_... ne retourne rien dans certains cas. Faudrait voir comment crispy se comporte avec ça, mais dans l'absolu, c'est pas très propre.

Crispy prend en compte cette possibilité et ignore les champs qui serait None. Je ne vois pas vraiment comment faire plus propre qu'actuellement sans complexifier inutilement le code.

@Situphen
Copy link
Member Author

Je suis plutôt confiant sur le bon fonctionnement de la déconnexion et du changement de pseudo ou d'adresse de courriel pour tous les membres (inscription classique et inscription via les réseaux sociaux), par contre je ne sais pas trop si la possibilité aux membres inscrits via les réseaux sociaux de se créer un mot de passe va poser un soucis ou pas. C'est un cas à tester sur la bêta avec attention.

@Situphen Situphen added the S-Régression Corrige un problème sur un composant qui fonctionnait auparavant label Nov 17, 2022
@DonKnacki
Copy link

j'ai testé avec un compte google: lorsqu'on se reconnecte après changement de mot de passe, cela ne doit pas le demander ? (ça le demande en revanche sur la page changement de login)
Avec un compte classique, c'est Ok.

@Situphen
Copy link
Member Author

j'ai testé avec un compte google: lorsqu'on se reconnecte après changement de mot de passe, cela ne doit pas le demander ? (ça le demande en revanche sur la page changement de login)
Avec un compte classique, c'est Ok.

En fait, le mot de passe actuel ne doit être demandé qu'aux comptes qui ont un mot de passe. On demande donc toujours le mot de passe actuel aux comptes inscrits de manière classique pour les changements de pseudo, courriel et mot de passe ainsi que pour la suppression du compte. On ne demande pas le mot de passe aux comptes inscrits via les réseaux sociaux, sauf s'ils ont créé un mot de passe. À partir du moment où un compte inscrit via les réseaux sociaux a créé un mot de passe, alors ce mot de passe est demandé pour les changements de pseudo, courriel et mot de passe ainsi que pour la suppression du compte.

@DonKnacki
Copy link

j'ai testé avec un compte google: lorsqu'on se reconnecte après changement de mot de passe, cela ne doit pas le demander ? (ça le demande en revanche sur la page changement de login)
Avec un compte classique, c'est Ok.

En fait, le mot de passe actuel ne doit être demandé qu'aux comptes qui ont un mot de passe. On demande donc toujours le mot de passe actuel aux comptes inscrits de manière classique pour les changements de pseudo, courriel et mot de passe ainsi que pour la suppression du compte. On ne demande pas le mot de passe aux comptes inscrits via les réseaux sociaux, sauf s'ils ont créé un mot de passe. À partir du moment où un compte inscrit via les réseaux sociaux a créé un mot de passe, alors ce mot de passe est demandé pour les changements de pseudo, courriel et mot de passe ainsi que pour la suppression du compte.

Dans ce cas, je valide : c'est bien le comportement que j'ai pu observer

@Situphen
Copy link
Member Author

Situphen commented Dec 1, 2022

Merci pour la QA !

@Arnaud-D
Copy link
Contributor

Arnaud-D commented Dec 1, 2022

Enfin, pour ce qui est du côté purement technique, la solution me convient.

QA OK ✔️

@Arnaud-D Arnaud-D merged commit 60ad124 into zestedesavoir:dev Dec 1, 2022
philippemilink pushed a commit that referenced this pull request Dec 4, 2022
@Situphen Situphen deleted the mdp branch March 10, 2024 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bloquant Ticket qui doit être traité avant la prochaine mise à jour C-Back Concerne le back-end Django S-Régression Corrige un problème sur un composant qui fonctionnait auparavant
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Impossible aux membres connectés avec un compte Google de se désinscrire et de créer un mot de passe
4 participants