ldap: prevent crash when recording a timeout failure (#73342) #9

bdauvergne · 2023-03-05T15:51:35+01:00

bdauvergne commented

2023-03-05 15:51:35 +01:00

J'ai fait le minimum pour cacher l'erreur mais je ne suis pas convaincu par le code lui même, on devrait dans tous les cas enregistrer quelque chose dans le journal.

bdauvergne added 1 commit 2023-03-05 15:51:36 +01:00

gitea/authentic/pipeline/head This commit looks good Details

6786b40583 ldap: prevent crash when recording a timeout failure (#73342 )

pmarillonnet commented

2023-03-15 11:26:10 +01:00

Tu veux dire logguer le fait qu’on tente un lookup sur des attributs inexistants ?

bdauvergne commented

2023-03-20 09:16:22 +01:00

Tu veux dire logguer le fait qu’on tente un lookup sur des attributs inexistants ?

Non pas du tout. Ici on est sur un échec d'authentification, à défaut d'avoir pu résoudre authentifier l'utilisateur on essaie de retrouver un minimum qui c'est mais comme c'est un timeout et pas juste un mauvais mot de passe, on ne peut pas interroger le LDAP pour trouver l'utilisateur et donc on ne log rien dans ce cas précis d'échec d'authentification.

Je ne suis pas convaincu par le code parce que je pense que c'est problèmes disparaîtrait si au lieu de faire l'authentification sur le LDAP avant la recherche du compte en base on faisait l'inverse, i.e. on cherche toujours d'abord username/email localement, si on trouve un compte relié au LDAP on l'authentifie. Déjà ça améliorerait les performances et les blocages, i.e. plus jamais d'appel au LDAP dans le cas général et on saurait toujours quel compte n'a pas pu s'authentifier.

> Tu veux dire logguer le fait qu’on tente un lookup sur des attributs inexistants ? Non pas du tout. Ici on est sur un échec d'authentification, à défaut d'avoir pu résoudre authentifier l'utilisateur on essaie de retrouver un minimum qui c'est mais comme c'est un timeout et pas juste un mauvais mot de passe, on ne peut pas interroger le LDAP pour trouver l'utilisateur et donc on ne log rien dans ce cas précis d'échec d'authentification. Je ne suis pas convaincu par le code parce que je pense que c'est problèmes disparaîtrait si au lieu de faire l'authentification sur le LDAP avant la recherche du compte en base on faisait l'inverse, i.e. on cherche toujours d'abord username/email localement, si on trouve un compte relié au LDAP on l'authentifie. Déjà ça améliorerait les performances et les blocages, i.e. plus jamais d'appel au LDAP dans le cas général et on saurait toujours quel compte n'a pas pu s'authentifier.

pmarillonnet commented

2023-03-22 15:00:56 +01:00

Non pas du tout. Ici on est sur un échec d'authentification, à défaut d'avoir pu résoudre authentifier l'utilisateur on essaie de retrouver un minimum qui c'est mais comme c'est un timeout et pas juste un mauvais mot de passe, on ne peut pas interroger le LDAP pour trouver l'utilisateur et donc on ne log rien dans ce cas précis d'échec d'authentification.

Je ne suis pas convaincu par le code parce que je pense que c'est problèmes disparaîtrait si au lieu de faire l'authentification sur le LDAP avant la recherche du compte en base on faisait l'inverse, i.e. on cherche toujours d'abord username/email localement, si on trouve un compte relié au LDAP on l'authentifie. Déjà ça améliorerait les performances et les blocages, i.e. plus jamais d'appel au LDAP dans le cas général et on saurait toujours quel compte n'a pas pu s'authentifier.

Merci pour la remise en contexte, c’est plus clair pour moi.
Pour ce qui est de faire passer la recherche locale avant l’authn distante, je vais regarder et faire un ticket. En attendant, ack ici.

> Non pas du tout. Ici on est sur un échec d'authentification, à défaut d'avoir pu résoudre authentifier l'utilisateur on essaie de retrouver un minimum qui c'est mais comme c'est un timeout et pas juste un mauvais mot de passe, on ne peut pas interroger le LDAP pour trouver l'utilisateur et donc on ne log rien dans ce cas précis d'échec d'authentification. > > Je ne suis pas convaincu par le code parce que je pense que c'est problèmes disparaîtrait si au lieu de faire l'authentification sur le LDAP avant la recherche du compte en base on faisait l'inverse, i.e. on cherche toujours d'abord username/email localement, si on trouve un compte relié au LDAP on l'authentifie. Déjà ça améliorerait les performances et les blocages, i.e. plus jamais d'appel au LDAP dans le cas général et on saurait toujours quel compte n'a pas pu s'authentifier. Merci pour la remise en contexte, c’est plus clair pour moi. Pour ce qui est de faire passer la recherche locale avant l’authn distante, je vais regarder et faire un ticket. En attendant, ack ici.

pmarillonnet approved these changes 2023-03-22 15:01:15 +01:00

pmarillonnet left a comment

Ok.

pmarillonnet commented

2023-03-22 15:08:36 +01:00

Pour ce qui est de faire passer la recherche locale avant l’authn distante, je vais regarder et faire un ticket.

(#75705)

> Pour ce qui est de faire passer la recherche locale avant l’authn distante, je vais regarder et faire un ticket. (#75705)

bdauvergne merged commit 1aeb81e3a1 into main

2023-03-24 15:37:42 +01:00

bdauvergne deleted branch wip/73342-ldap-erreur-log-erreur-TypeError

2023-03-24 15:37:42 +01:00

Sign in to join this conversation.

No reviewers

No Label

No Milestone

No Assignees

2 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/authentic#9