Produits Entr'ouvert » Authentic 2

Le patch ci-joint implémente la gestion de PasswordPolicyControl en cas de modification ou de réinitialisation du mot de passe. Comme proposé, cela reste conditionné à l'activation du paramètre de configuration use_controls (au niveau du block), comme c'est le cas pour le support de l'authentification.

Benjamin Dauvergne a écrit :

• j'ai l'impression qu'il y a la correction d'un bug dans le tas :
  [...]
  si cet encodage est nécessaire il faut le séparer dans un autre patch

Effectivement, contrairement au cas avec un AD, le mot de passe n'était pas encodé, python-ldap requiert un bytes-string. J'ai fait un patch dédié pour ça.

• il faudrait découper ça en 3 patchs:
  • renommage process_controls -> process_bind_controls
    • à relire je n'ai pas l'impression qu'il y ait de différence entre les deux méthodes à part le message de log, je me trompe ? Dans ce cas autant rendre le message variable et ne garder qu'une méthode,

Pas exactement, mais c'est pas loin effectivement : dans le cas du login, on gère la request et on affiche le message d'erreur via messages.add_message() en plus d'activer request.needs_password_change dans certains cas. Dans le cas d'une modification de mot de passe, on affiche l'erreur simplement en levant une exception PasswordChangeError.

J'ai mis le renommage dans un commit dédié.

• correction citée plus haut
• enfin l'ajout des contrôles, il faudrait factoriser ces lignes entre les différentes méthodes de modification

C'est fait également et dans un commit dédié.

Il faudrait des tests.

J'ai ajouté des tests via un commit dédié pour chacun des messages d'erreurs gérés sauf changeAfterReset que je ne vois pas trop comment provoquer.

Au passage, j'ai également ajouté des patchs pour :

• utiliser les variables USERNAME & PASS un peu partout dans les tests LDAP au lieu de dupliquer des valeurs hard-codées
• corriger la gestion des erreurs passwordExpired (son code vaut zéro et il fallait un donc faire un test à coup de is not None)

Note : je précise que les tests sont prévus pour fonctionner une fois l'ensemble des autres patchs intégrés, y compris ceux de #69468, #69466 et #69464.

Benjamin Renard a écrit :

J'ai ajouté des tests via un commit dédié pour chacun des messages d'erreurs gérés sauf changeAfterReset que je ne vois pas trop comment provoquer.

Après réflexion, j'ai trouvé un moyen de tester cette erreur : il faut bloquer le compte à force de connexion avec un mauvais mot de passe, débloquer le compte comme un admin le ferait en spécifiant pwdReset à TRUE et retenter une connexion avec le bon mot de passe cette fois-ci.

En implémentant ce test, je me suis par ailleurs rendu compte que dans cette situation, on devrait provoquer une redirection vers le formulaire de réinitialisation du mot de passe et non laisser l'utilisateur sur le formulaire de connexion. J'ai donc implémenté cela et ajuster mon test en conséquence.

Note : À ce sujet, je suis pas sûre que j'utilise la bonne manière pour détecter la next URL lors de la redirection vers le formulaire de réinitialisation du mot de passe.