Produits Entr'ouvert » Authentic 2

(ou plus simplement si j'avais eu un peu plus de logs)

On confond les deux cas différents où (i) l’usager n’est plus dans le groupe et (ii) le DN du groupe ne correspond pas à une entrée connue du LDAP.

(Cf. quelques lignes plus bas, en fonction de l’attribution du rôle, on peut tomber dans :

                # Remove extra roles
                elif group_dn not in group_dns and role in roles:
                    user.roles.remove(role)

Est-ce qu’il y aurait moyen de faire, à l’initialisation du backend, un search_s sur le nœud seul (pas le sous-arbre) pour chacune des clés de group_to_role_mapping et voir si ça correspond à quelque chose ?

Paul Marillonnet a écrit :

Est-ce qu’il y aurait moyen de faire, à l’initialisation du backend, un search_s sur le nœud seul (pas le sous-arbre) pour chacune des clés de group_to_role_mapping et voir si ça correspond à quelque chose ?

Oui c'est exactement ça, l'idée c'est de récupérer la totalité des groupes de temps en temps et de vérifier que les groupes pointés dans la configuration en font partie. En pseudo-code :

group_dns = set(dn for dn, attributes in self.normalize_results(conn.search_s(group_basedn, ldap.SCOPE_SUBTREE, group_filter, ['1.1'])  # 1.1 is special attribute meaning, "no attribute requested" 
for dn in self.gather_all_group_dns_from_config():
    if dn not in group_dns:
        log.warning('ldap: unknown group %s referenced in configuration', dn)

Je ne sais pas si on veut faire ça tout le temps ou juste lors des synchros par exemple.

Je note qu’on change de fonctionnalité par rapport à la description du ticket, on passe de (i) "avertissement sur les groupes inexistants déclarés dans la config de mapping" à (ii) "avertissement sur les groupes déclarés dans la config de mapping mais qui ne sont pas dans le sous-arbre du group_basedn". Je ne sais pas si c’est ce qu’on veut.

Autre chose encore, je pense qu’indépendamment du choix de partir vers (i) ou (ii) il faut placer cette vérification plus tôt dans le backend, pas dans une méthode qui est appelée à chaque authentification d’un usager. Si — (i) seulement — le group_to_role_mapping est conséquent voire — (i) ou (ii) au choix — le group_basedn contient beaucoup de groupes, ça va taper sévère niveau requêtes.

Serghei Mihai a écrit :

Paul Marillonnet a écrit :

Je note qu’on change de fonctionnalité par rapport à la description du ticket, on passe de (i) "avertissement sur les groupes inexistants déclarés dans la config de mapping" à (ii) "avertissement sur les groupes déclarés dans la config de mapping mais qui ne sont pas dans le sous-arbre du group_basedn". Je ne sais pas si c’est ce qu’on veut.

Ok, un check lors de la lecture de la config et s'il y a group_to_role_mapping de défini, on vérifie si les DNs existent dans l'annuaire.

Pour le point 1 de Paul, je ne comprends pas la différence entre i et ii, pour moi c'est la même chose.

Pour le point 2 de Paul, tu n'a pas apporté de changement non plus, car get_config() est de toute façon appelé à chaque authentification. Je penche plutôt pour une validation faite dans sync-ldap-users via get_users(), ça fera un warning par heure au pire. En attendant une configuration graphique pour afficher les erreurs c'est suffisant.

Benjamin Dauvergne a écrit :

Pour le point 2 de Paul, tu n'a pas apporté de changement non plus, car get_config() est de toute façon appelé à chaque authentification. Je penche plutôt pour une validation faite dans sync-ldap-users via get_users(), ça fera un warning par heure au pire.

Tu penses juste au cas des synchros toutes les heures, or il y a aussi celui où l'usager se connecte et qu'à ce moment un rôle déclaré dans la conf n'existe pas. C'est le cas couvert par le test. Mais ok, on peut se dire que lever le warning uniquement lors de sync-ldap-users suffit.

En attendant une configuration graphique pour afficher les erreurs c'est suffisant.

Cela ne nous dispense pas de logguer le warning car une fois l'annuaire configuré dans le manager, on va pas y retourner tous les jours pour voir s'il y a des éventuelles alertes.