Project

General

Profile

Development #66416

Ajout du support de ppolicy lors de la modification du mot de passe

Added by Benjamin Renard 9 months ago. Updated 4 months ago.

Status:
Solution proposée
Priority:
Normal
Category:
-
Target version:
-
Start date:
20 June 2022
Due date:
% Done:

0%

Estimated time:
Patch proposed:
No
Planning:
No

Description

Nous avions par le passé ajouté le support de ppolicy lors de la connexion et autre client nous demande aujourd'hui de l'ajouter lors du changement de mot de passe (changement & réinitialisation). Aujourd'hui, en cas de mot de passe refusé par l'annuaire LDAP (du fait de ppolicy ou non d'ailleurs), Authentic renvoi une erreur 500 sans précision. L'idée serait ici d'ajouter ici le contrôle PasswordPolicyControl lors de la modification du mot de passe d'un utilisateur (LDAPUser.set_password) comme cela avait été fait lors de la connexion (LDAPBackend.authenticate_block) de manière paramétrable via le paramètre use_controls existant.


Files

History

#1

Updated by Benjamin Renard 9 months ago

J'ai commencé à regarder le sujet et cela ne semble pas très compliqué. La seule problématique qui me pose problème c'est de savoir remonter l'erreur correctement à l'utilisateur. En effet, la méthode set_password étant appelée par la méthode save des formulaires Django et il ne suffit donc pas de déclencher simplement une exception ValidationError. J'ai fait quelques recherches sur le sujet et je n'ai pas trouvé de solution simple et propre documentée pour cela. Avez-vous une idée de comment faire ?

PS : dans l'idéal, on aurait pu tenter d'utiliser la fonctionnalité LDAPv3 Grouping of Related Operations pour implémenter une validation via form.clean avant form.save, mais cela n'est pas encore supporté par python ldap3 (et j'ai pas trouvé si c'était le cas par OpenLDAP).

#2

Updated by Benjamin Renard 9 months ago

Benjamin Renard a écrit :

J'ai commencé à regarder le sujet et cela ne semble pas très compliqué. La seule problématique qui me pose problème c'est de savoir remonter l'erreur correctement à l'utilisateur. En effet, la méthode set_password étant appelée par la méthode save des formulaires Django et il ne suffit donc pas de déclencher simplement une exception ValidationError. J'ai fait quelques recherches sur le sujet et je n'ai pas trouvé de solution simple et propre documentée pour cela. Avez-vous une idée de comment faire ?

Je me permets une petite relance sur le sujet : une idée pour ma problématique exposée ci-dessus ?

#3

Updated by Benjamin Renard 6 months ago

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.

#4

Updated by Benjamin Dauvergne 5 months ago

  • j'ai l'impression qu'il y a la correction d'un bug dans le tas :
                    modlist = [(ldap.MOD_REPLACE, key, [new_password.encode('utf-8')])]
    

    si cet encodage est nécessaire il faut le séparer dans un autre patch
  • 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éthode à part le message de log, je me trompe ? Dans ce cas autant rendre le message variable et ne garder qu'une méthode,
    • correction citée plus haut
    • enfin l'ajout des contrôles, il faudrait factoriser ces lignes entre les différentes méthodes de modification

Il faudrait des tests.

#5

Updated by Benjamin Renard 5 months ago

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.

#6

Updated by Benjamin Renard 5 months ago

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.

#7

Updated by Benjamin Dauvergne 4 months ago

  • Status changed from Nouveau to Solution proposée
  • Assignee set to Benjamin Renard
#8

Updated by Benjamin Renard 4 months ago

Je me permets une petite relance sur le sujet. Notre client nous relance sur le sujet.

Also available in: Atom PDF