Produits Entr'ouvert » Authentic 2

Je ne sais pas quelle approche on peut prendre ici, rendre le comportement optionnel ? faire un .filter(...).first() plutôt que le .get(...) ?

Tester d'abord en prêtant attention à la casse, et seulement si ça ne donne rien tenter sans ?

Frédéric Péters a écrit :

Tester d'abord en prêtant attention à la casse, et seulement si ça ne donne rien tenter sans ?

J'aime bien cette idée là (donc 0001-ldap-lookup-users-with-exact-case-before-going-case-.patch a ma préférence)

Ceci dit l'erreur qu'on a c'est MultipleObjectsReturned : il faut éviter d'utiliser get, plutôt prendre l'élément le plus récent, selon la date de dernière connexion puis la date de création.

+1 pour faire une comparaison en 2 étapes (avec et sans casse).

u.delete()

→ other.delete().

Fonctionnellement je pense que ça passe (même si la magie me fait toujours un peu peur, mais c'est moi).

Passer le log.debug("found %d users ...") en log.info

Question sur le order_by(-last_login) : voir comment ça réagit quand il n'y a pas encore de last_login (parce que blank=True), typiquement pour les comptes qui viennent d'être synchronisés et sont donc ceux qu'il faut utiliser.

https://www.postgresql.org/docs/8.3/static/queries-order.html :

The NULLS FIRST and NULLS LAST options can be used to determine whether nulls appear before or after non-null values in the sort ordering. By default, null values sort as if larger than any non-null value; that is, NULLS FIRST is the default for DESC order, and NULLS LAST otherwise.

Pas de nulls last dans Django, et je ne crois pas qu'on puisse trier par __isnull avant, enfin j'ai rien vu qui le confirme sur le net, donc oui faudra trier à la paluche après.

Il reste:

                    log.debug('found %d users, collectings roles into the first one and deleting the other ones.',
                              len(users))

il faut un len(users).

Et:

E     File "/home/serghei/dev/authentic/src/authentic2/backends/ldap_backend.py", line 844
E       key=lambda u: u is not None, u.last_login)
E   SyntaxError: non-keyword arg after keyword arg

Je pense qu'il faut mettre les None en premier (utilisateur jamais connecté = compte tout nouveau, par ex. vient juste d'être synchronisé, et c'est celui ci qui est le bon pour l'avenir). Donc virer le reserved=True du sorted. IMveryveryHO seulement.

Thomas Noël a écrit :

Je pense qu'il faut mettre les None en premier (utilisateur jamais connecté = compte tout nouveau, par ex. vient juste d'être synchronisé, et c'est celui ci qui est le bon pour l'avenir). Donc virer le reserved=True du sorted. IMveryveryHO seulement.

En local avec un a2 qui tape dans une base Postgres je crée un nouvel utilisateur sans me connecter avec.
Ça donne:

In [29]: for u in User.objects.all().order_by('-last_login'):
    ...:     print u.username, u.last_login
    ...:     
NeverLogguedUser None
neverloggueduser 2018-11-05 22:30:13.853561+00:00

J'ai bien l'impression qu'on aura celui qui ne s'est jamais connecté sans faire de tri manuel.

Serghei Mihai a écrit :

J'ai bien l'impression qu'on aura celui qui ne s'est jamais connecté sans faire de tri manuel.

Comme a dit Benjamin en note 14, on n'en sait rien, ça dépend du sgbd, la requête SQL générée par Django ne précise rien (pour pgsql pas de "NULLS FIRST" ou "NULLS LAST" par exemple).

Serghei Mihai a écrit :

Ok, tri avec le last_login à None toujours en premier. Et le log passé en info.

Ni l'un ni l'autre, sans doute un mauvais patch.

Arf, my bad, pour le coup le « users = sorted(users, key=lambda u: (u.last_login is not None, u.last_login)) » met les dates dans le mauvais sens. Il faut bien un reversed et donc remplacer is not par is.

(et à force je me dis qu'il faudrait s'embêter à faire un test je pense :/ )

Thomas Noël a écrit :

Arf, my bad, pour le coup le « users = sorted(users, key=lambda u: (u.last_login is not None, u.last_login)) » met les dates dans le mauvais sens. Il faut bien un reversed et donc remplacer is not par is.

• True, 2018-01-01
• False, None
• True, 2018-01-02

se trie en "reversed" (le tri par défaut c'est croissant, donc là c'est décroissant):

• True, 2018-01-02
• True, 2018-01-01
• False, None

N'est-ce pas ce que l'on veut ?

In [1]: l = [
   ...: (True, 2),
   ...: (False, None),
   ...: (True, 1)]

In [2]: sorted(l, reverse=True)
Out[2]: [(True, 2), (True, 1), (False, None)]

Benjamin Dauvergne a écrit :

Thomas Noël a écrit :

Arf, my bad, pour le coup le « users = sorted(users, key=lambda u: (u.last_login is not None, u.last_login)) » met les dates dans le mauvais sens. Il faut bien un reversed et donc remplacer is not par is.

Hmm, trié True vient après False (1 > 0), donc ça :

• True, 2018-01-01
• False, None
• True, 2018-01-02

Sauf si on change de u.last_login is not None en u.last_login is None.

Serghei Mihai a écrit :

Benjamin Dauvergne a écrit :

Thomas Noël a écrit :

Arf, my bad, pour le coup le « users = sorted(users, key=lambda u: (u.last_login is not None, u.last_login)) » met les dates dans le mauvais sens. Il faut bien un reversed et donc remplacer is not par is.

Hmm, trié True vient après False (1 > 0), donc ça :

• True, 2018-01-01
• False, None
• True, 2018-01-02

Sauf si on change de u.last_login is not None en u.last_login is None.

Les valeurs que je donne sont bien avec u.last_login is not None il me semble.

Si on modifie la fonction lambda en u.last_login is None, avec le tri inverse les comptes qui ne se sont jamais connectés remontent en premier.

Serghei Mihai a écrit :

Si on modifie la fonction lambda en u.last_login is None, avec le tri inverse les comptes qui ne se sont jamais connectés remontent en premier.

Mais comme il y a is not None dans mon code je ne vois pas le besoin de se poser la question.

Au cas où je suis toujours mentionné comme auteur alors que je n'en suis responsable que des premières lignes au tout début.

C'est is not None et pas is None ensuite le test ne semble pas tester le code en question vu qu'il n'y a pas de last_login de défini sur ces utilisateurs.

Benjamin Dauvergne a écrit :

C'est is not None et pas is None ensuite le test ne semble pas tester le code en question vu qu'il n'y a pas de last_login de défini sur ces utilisateurs.

Erreur de ma part, il y a bien un test sur le last_login mais justement c'est le user avec last_login = None qui est conservé et on préfère conserver celui qui a éventuellement déjà été utilisé.

Serghei Mihai a écrit :

On a des cas inverses d'Alfortville: à Villeurbanne le compte le plus ancien est le bon (avec les bons rôles). Mais c'est une recette, on s'en fout.
Allons sur la conservation du compte le plus récent.

Il n'y a pas de problèmes sur les rôles puisqu'on les garde tous.

Sur jenkins les tests échouent au niveau de RBAC:

>       assert float(t) / 1000 < 0.008
E       assert (8.29203200340271 / 1000) < 0.008
E        +  where 8.29203200340271 = float(8.29203200340271)

tests_rbac/test_rbac.py:107: AssertionError

mais tout est vert chez moi en local.

Il devait manquer le commit #27661 dans ta branche non ?

C'était ça. J'ai refait la branche est poussé.
Jenkins bosse.

C'est bon niveau jenkins.