Produits Entr'ouvert » Authentic 2

Concernant les attributs états, je pense utile de définir ce que nous pourrions en faire :

• pwdChangedTime : je vois pas bien quand cela pourrait être utile de le récupérer, peut-être lors de l'affichage du message d'expiration du mot de passe ("Vous devez changer votre mot de passe (dernière modification: XXX)")
• pwdFailureTime : utile en cas d'échec de connexion et de mot de passe invalide (on affiche le nombre d'échecs passés)
• pwdGraceUseTime : utile pour afficher le nombre d'authentifications autorisées restantes
• pwdHistory : je vois pas quand on pourrait en avoir besoin : aucun intérêt à l'affichage et l'historique est complètement géré par le serveur LDAP. Il ne devrait pas être récupéré selon moi.
• pwdReset : à la connexion, si pwdReset vaut TRUE , il doit être renvoyé vers la modification de son mot de passe.
• pwdUniqueAttempts : attribut non-standard, il ne devrait pas être récupéré.

Par ailleurs, il doit dans certaine situation être possible d'économiser une requête LDAP spécifique pour récupérer ces attributs : lors d'une connexion avec le DN de l'utilisateur, on commence par récupérer son DN à partir du login fourni et pour ça, on fait une première requête avec le compte de service. Lors de cette requête, on doit pouvoir récupérer les attributs d'états. Il faudrait cependant prévoir un cas spécifique lorsque cette première connexion n'est pas nécessaire (paramètres user_dn_template et bind_with_username).

Par ailleurs, il doit dans certaine situation être possible d'économiser une requête LDAP spécifique ...

En effet et merci de m'avoir montré que ça se passait par ici

Less problèmes relevés par Benjamin Renard pour archive:

• la variable ppolicy_dn est mal nommé, cela devrait plutôt être ppolicy_default_dn et surtout, celle peut-être écrasé utilisateur par utilisateur via l'attribut pwdPolicySubentry
• 'The password expired after {pwdmaxage}' => ici pwdmaxage est un entier, ce qui rend le message certainement incompréhensible pour un user normal

Pour éviter toute confusion, je vais attendre une revue de code du patch actuel avant d'aller plus loin. Il n'y a pas le feu au lac, c'est juste pour dire :-)

Je préférerai un ticket global montrant l'usage de l'API introduite, mais tu peux garder celui-ci et avancer sur la suite,...

J'ai créé https://dev.entrouvert.org/issues/51653 dans l'idée de faire un ticket global. Est-ce que c'est ce que tu avais en tête ou bien je suis à coté de la plaque (très possible ;-) ?

Est-ce que le patch proposé ici convient ? https://dev.entrouvert.org/attachments/52068

Sinon je peux le corriger.

Il y avait plein de conflits parce qu'on a passé le code à la moulinette de black et isort (#52457), il faudrait désormais que tes patches soient conformes : pour cela on a un hook pre-commit de dispo, c'est expliqué dans la section « Code Style » du README.

Je suis vraiment désolé de vous avoir occasionné ce travail supplémentaire. Toutes mes excuses :-(

• Enlever le français du message de commit, il a été tapé dans le ticket et c'est très bien comme ça.
• Ne pas rajouter un .format(**attributes) sur les lignes qui n'en ont pas besoin, ça aura aussi l'avantage d'avoir un diff plus light.
• Éviter le time.sleep dans les tests, voir plutôt du côté de la fixture freezer pour ces cas d'usages.
• Il y a un print(attributes_results) qui j'imagine est du debug, à retirer.
• Certaines lignes paraissent vraiment longues, est-ce que black a bien tourné sur le code ?

Loïc Dachary a écrit :

• time.sleep a été laissé parce que le délai concerne le serveur ldap qui implémente la ppolicy et que la fixture n'a pas de contrôle a ce niveau

Une attente de 3 secondes ne nous tuera pas, mais juste pour indication: pour des tests plus réalistes du même genre il y a cet outil https://github.com/wolfcw/libfaketime . C'est dispo dans Debian (paquet faketime), à voir si ça serait utilisable avec ldaptools pour lancer slapd (le test peut-être conditionné à la présence de faketime sur le système).

Je ne comprends pas le rapport entre le changement de la valeur par défaut de use_control et cette modification ? Je veux dire par la que cette modification ne concerne qu'un chemin de code qui est actif si use_control = True. Ou bien j'ai raté un truc ?

Jusqu'à présent use_controls ne faisait qu'ajouter des contrôles à des requêtes existantes (ou consulter les contrôles sur les retours à simple_bind) mais là ça va générer des requêtes inutiles qui pourrait planter/etc sur d'autres implémentations que OpenLDAP (pour des raisons imprévisibles). De toute façon ça n'a pas d'impact de le désactiver.

De toute façon ça n'a pas d'impact de le désactiver.

Ca inverse la valeur par défaut et donc le comportement de tous les déploiement qui comptent sur cette valeur à la prochaine mise à jour.

commit ff9e90ce5865efc512cdcf0d53a9c847451e38df (HEAD -> main, origin/main)
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Tue May 18 18:10:14 2021 +0200

    ldap: default use_controls to False (#51239)

commit 1f6378256e804f04ac524d89d142f61b014c2466
Author: Loïc Dachary <ldachary@easter-eggs.com>
Date:   Fri Feb 12 19:02:26 2021 +0100

    ldap: add method to get ppolicy operational attributes (#51239)

    Fixes: #51239

    License: MIT