Development #48136
auth_fc : variation sur django.contrib.auth.hashers.is_password_usable entre django 1.11 et django 2 fait planter les tests
0%
Fichiers
Révisions associées
Historique
Mis à jour par Paul Marillonnet il y a plus de 3 ans
- Fichier 0001-custom_user-add-User.has_usable_password-48136.patch 0001-custom_user-add-User.has_usable_password-48136.patch ajouté
- Statut changé de Nouveau à Solution proposée
- Patch proposed changé de Non à Oui
Mis à jour par Paul Marillonnet il y a plus de 3 ans
- Fichier 0001-custom_user-add-User.has_usable_password-48136.patch 0001-custom_user-add-User.has_usable_password-48136.patch ajouté
Oups, des trucs en cache local pas inclus dans le commit. Corrigé ici.
Mis à jour par Thomas Noël il y a plus de 3 ans
Je trouverais plus clair (et plus propre) de commencer plutôt par un « if not super().has_usable_password(): return False » qu'on comprenne qu'on ajoute juste des vérifications en plus. Non ?
Mis à jour par Paul Marillonnet il y a plus de 3 ans
- Fichier 0001-custom_user-add-User.has_usable_password-48136.patch 0001-custom_user-add-User.has_usable_password-48136.patch ajouté
- Fichier 0002-auth_fc-misc-django-2-compatibility-48136.patch 0002-auth_fc-misc-django-2-compatibility-48136.patch ajouté
Thomas Noël a écrit :
Je trouverais plus clair (et plus propre) de commencer plutôt par un « if not super().has_usable_password(): return False » qu'on comprenne qu'on ajoute juste des vérifications en plus. Non ?
Ok très bien.
Mis à jour par Thomas Noël il y a plus de 3 ans
Paul Marillonnet a écrit :
Thomas Noël a écrit :
Je trouverais plus clair (et plus propre) de commencer plutôt par un « if not super().has_usable_password(): return False » qu'on comprenne qu'on ajoute juste des vérifications en plus. Non ?
Ok très bien.
Heu, pourquoi ça devient deux patches ?
Et ma proposition c'était de remplacer le « if (self.password is None or self.password.startswith(UNUSABLE_PASSWORD_PREFIX)): return False » par le super.
Mais bon en vrai je ne comprends pas très bien pourquoi on veut modifier ça. Suivre Django me semblerait une meilleure idée (et donc comprendre pourquoi ils ont fait évoluer leur code)
Mis à jour par Paul Marillonnet il y a plus de 3 ans
- Statut changé de Solution proposée à En cours
- Patch proposed changé de Oui à Non
Thomas Noël a écrit :
Heu, pourquoi ça devient deux patches ?
Je voyais pas ce que la correction sur is_authenticated avait en rapport, mais ok je peux tout regrouper.
Et ma proposition c'était de remplacer le « if (self.password is None or self.password.startswith(UNUSABLE_PASSWORD_PREFIX)): return False » par le super.
Ok mais le souci c'est qu'en django 2 le is_password_usable (appelé par django.contrib.auth.AbstractBaseUser) change et devient :
def is_password_usable(encoded):
"""
Return True if this password wasn't generated by
User.set_unusable_password(), i.e. make_password(None).
"""
return encoded is None or not encoded.startswith(UNUSABLE_PASSWORD_PREFIX)
Mais bon en vrai je ne comprends pas très bien pourquoi on veut modifier ça. Suivre Django me semblerait une meilleure idée (et donc comprendre pourquoi ils ont fait évoluer leur code)
Et donc oui ok je suis d'accord, je vais retrouver le ticket côté django pour comprendre ce qui se trame derrière ce changement.
Mis à jour par Paul Marillonnet il y a plus de 3 ans
Paul Marillonnet a écrit :
Et donc oui ok je suis d'accord, je vais retrouver le ticket côté django pour comprendre ce qui se trame derrière ce changement.
C'est https://code.djangoproject.com/ticket/28718. Ce qui plante c'est que Django 2 considère qu'un mot de passe valant None est utilisable car il n'est pas l'image de User.set_unusable_password().
Il faut corriger ça dans a2.
Mis à jour par Benjamin Dauvergne il y a plus de 3 ans
Ok donc ce qu'il faut :
1. faire le tour des endroits où on crée un utilisateur sans mot de passe, et y ajouter un .set_unusable_password()
2. une migration pour faire set_unusable_password() sur tous les utilisateurs avec un mot de passe vide
Pour rester concentrer sur auth_fc on peut ne le faire que dans son cas ici et ouvrir un ticket plus large ensuite (mais je pense que ça concerne uniquement auth_saml, auth_oidc, auth_ldap et éventuellement authentic2-beid qu'il faut regarder). Ça concerne aussi django-mellon.
Mis à jour par Frédéric Péters il y a plus de 3 ans
authentic2-beid qu'il faut regarder
non ce module est obsolète.
Mis à jour par Benjamin Dauvergne il y a plus de 3 ans
Frédéric Péters a écrit :
authentic2-beid qu'il faut regarder
non ce module est obsolète.
Ok, authentic2-auth-fedict.
Mis à jour par Benjamin Dauvergne il y a plus de 3 ans
- Fichier 0001-misc-set-unusable-password-on-federated-users-48136.patch 0001-misc-set-unusable-password-on-federated-users-48136.patch ajouté
- Fichier 0002-temporary-add-authentic-py3-dj22-drf39-to-Jenkinsfil.patch 0002-temporary-add-authentic-py3-dj22-drf39-to-Jenkinsfil.patch ajouté
- Tracker changé de Bug à Development
- Statut changé de En cours à Solution proposée
- Patch proposed changé de Non à Oui
Mis à jour par Benjamin Dauvergne il y a plus de 3 ans
- Fichier 0001-misc-set-unusable-password-on-federated-users-48136.patch 0001-misc-set-unusable-password-on-federated-users-48136.patch ajouté
- Fichier 0002-temporary-add-authentic-py3-dj22-drf39-to-Jenkinsfil.patch 0002-temporary-add-authentic-py3-dj22-drf39-to-Jenkinsfil.patch ajouté
Comme d'ab, j'oublie la migration.
Mis à jour par Serghei Mihai il y a plus de 3 ans
- Statut changé de Solution proposée à Solution validée
Mis à jour par Benjamin Dauvergne il y a plus de 3 ans
- Statut changé de Solution validée à Résolu (à déployer)
commit 7e013975f70a227256335d7de9cc3396f0137d35
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date: Tue Nov 3 09:54:41 2020 +0100
misc: set unusable password on federated users (#48136)Mis à jour par Frédéric Péters il y a plus de 3 ans
- Statut changé de Résolu (à déployer) à Solution déployée
misc: set unusable password on federated users (#48136)