Projet

Général

Profil

Development #19365

Attributs issus d'informations d'objets LDAP en relation avec l'utilisateur connecté

Ajouté par Benjamin Renard il y a plus de 6 ans. Mis à jour il y a plus de 3 ans.

Statut:
Fermé
Priorité:
Normal
Assigné à:
Catégorie:
-
Version cible:
-
Début:
11 octobre 2017
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Oui
Planning:
Non

Description

Le principe :
Nous avons besoin de fournir des attributs SAML alimentés sur la base d'informations d'objet(s) LDAP en relation avec l'utilisateur connecté.

La subtilité :
On peut donc avoir plusieurs objets en relation pour lesquels on veut récupérer une ou plusieurs informations.

Exemple :
  • Un utilisateur est associé à une ou plusieurs agences
  • Pour chacune de ses agences, on veut retourner son nom, son adresse et son mail

Fichiers

Révisions associées

Révision 7ac1fbcb (diff)
Ajouté par Benjamin Renard il y a plus de 3 ans

Manage LDAP extra attributes (#19365)

This extra attributes are retreived by making other LDAP queries
with parameters composed by looping on an user object's attribute
values. A mapping will be apply on corresponding objects's attributes
and resulting informations are compiled in a list and serialize in
JSON (configurable, but JSON is the only format available for now)

Historique

#1

Mis à jour par Benjamin Renard il y a plus de 6 ans

Reprise d'un échange par mail avec Benjamin Dauvergne :

Le 11/10/2017 à 10:52, Benjamin Dauvergne a écrit :
> Première remarque, il faut discuter via un ticket sur authentic, comme ça tout
> le monde joue chez nous.
> 
> La façon dont je vois la chose ce serait d'adopter un peu l'approche de ce
> commit sur ldapsaisie:
> 
>     commit 9add9c332148469fa6800eabacf033fc7f5c0438
>     Author: Benjamin Dauvergne<bdauvergne@entrouvert.com>
>     Date:   Sat Dec 13 12:15:37 2014 +0100
> 
>         Allow a sequence of filters in LSobjects profile configurations
>     
>         It's now possible for example to define a profile on an LSobject whose
>         attribute would contain the DN of a group the user is member of instead
>         of directly the dn of the user. A possible configuation using this new feature:
>     
>           'LSprofile' => array(
>         'admin' => array(
>           'LSobjects' => array(
>             'LSsupannGroupAdminByGroup' => array(
>               'filters' => array(
>             array(
>               'basedn' => $basedn,
>               'attr' => 'member',
>               'attr_value' => '%{dn}',
>               'LSobject' => 'LSsupannGroup',
>             ),
>             array(
>               'basedn' => $basedn,
>               'attr' => 'supannGroupeAdminDN',
>               'attr_value' => '%{dn}',
>               'LSobject' => 'LSsupannGroup',
>             )
>               ),
>             ),
>           ),
>         ),
>           )
> 
> On aurait une config JSON de ce type, si on suppose que l'utilisateur possède
> un attribut agenceDN qui pointe vers des objets de classe agence qui ont des
> attributs agenceName et agenceAdresse (j'invente tout mais c'est pour donner
> une idée, tu pourrais donner une forme qui te parait plus jolie/pertinente pour tout ça):
> 
> 
>     'extra_attributes': {
>        'agencies': {
>          'loop_over_attribute': 'agenceDN',
>          'filter': 'objectClass=agence',
>          'baseDN': '{agenceDN}',
>          'scope': 'base',
>          'mapping': {
>         'name': 'agenceName',
>         'address': 'agenceAdresse',
>          }
>     }
> 
> et donc on se retrouverait avec un attribut nommé 'agency' contenant une liste de dico de la forme
> 
>     [
>        {
>           'name': 'blabla',
>           'adress': 'coincoin',
>        },
>        ...
>     ]
> 
> et qu'on pourra référencer dans la configuration SAML des attributs.
> 
> Le problème suivant c'est qu'on ne supporte que des valeurs d'attribut auxquels
> on peut appliquer:
> 
>     value.encode('utf-8')
> 
> il faudrait voir comment on fait pour passer ça autrement. Comment vois-tu le
> transfert de cette structure dans un attribut SAML, juste en y mettant la
> sérialisation JSON ?
> 
> (Si tu peux recopier tout ça dans un ticket avant de répondre, c'est wunderbar:)  )
> 
> Le 10/10, Benjamin Renard a écrit :
>> [...]
>>
>> Comment voit-tu la meilleur solution pour implémenter cela dans Authentic :
>> Créons-nous un model Django pour gérer des objets en relation et pour chacun
>> de ces objets, on gère une liste d'attribut LDAP mappés ? Par ailleurs,
>> concernant le formatage de ces informations dans les attributs SAML, comment
>> voit tu la chose pour gérer les multiples informations pour chacun des
>> objets en relation ? Le protocole SAML prévoit quelques choses pour ça ou il
>> faut qu'on hash les infos via JSON (ou équivalent) ?
#2

Mis à jour par Benjamin Renard il y a plus de 6 ans

Je vois bien le principe que tu propose et ça me parait assez clair. Seul subtilité que je vois mais qui ne remet pas en cause ta solution : l'attribut de l'utilisateur qui référence le lien avec d'autres objets LDAP ne stockera pas forcément le DN de ces objets, il pourra par exemple contenir un identifiant unique à l'aide duquel il faudra composer un filtre pour rechercher l'objet en relation. Il faudra donc je pense prévoir que les paramètres filter et baseDN puissent utiliser en mots-clé substituables :
  • le nom d'un attribut de l'objet LDAP de l'utilisateur
  • le DN de l'objet LDAP de l'utilisateur
  • la valeur de l'attribut sur lequel on boucle

Pour la sérialisation des informations récupérées, je pensais effectivement à JSON si le protocole SAML ne proposait rien de mieux (attributs "structurés" ...?). Au vue de ta réponse, je comprend que non et donc JSON me semble à défaut une bonne solution.

Pour l'encoding UTF8, cela ne devrait pas poser de problèmes tant qu'on retourne bien un unicode du hash JSON des informations ?

#3

Mis à jour par Benjamin Dauvergne il y a plus de 6 ans

Benjamin Renard a écrit :

Je vois bien le principe que tu propose et ça me parait assez clair. Seul subtilité que je vois mais qui ne remet pas en cause ta solution : l'attribut de l'utilisateur qui référence le lien avec d'autres objets LDAP ne stockera pas forcément le DN de ces objets, il pourra par exemple contenir un identifiant unique à l'aide duquel il faudra composer un filtre pour rechercher l'objet en relation. Il faudra donc je pense prévoir que les paramètres filter et baseDN puissent utiliser en mots-clé substituables :
  • le nom d'un attribut de l'objet LDAP de l'utilisateur
  • le DN de l'objet LDAP de l'utilisateur
  • la valeur de l'attribut sur lequel on boucle

D'ac en plus ça me semble être le comportement de ldapsaisie justement.

Pour la sérialisation des informations récupérées, je pensais effectivement à JSON si le protocole SAML ne proposait rien de mieux (attributs "structurés" ...?). Au vue de ta réponse, je comprend que non et donc JSON me semble à défaut une bonne solution.

Attribut structuré ça veut dire définir un schéma XML, convertir dans ce ce format et parsé ça coté client, autant dire la galère.

Pour l'encoding UTF8, cela ne devrait pas poser de problèmes tant qu'on retourne bien un unicode du hash JSON des informations ?

Oui ça veut juste dire qu'au lieu de :

value.encode('utf-8')

on aura

if isinstance(value, dict):
    value = json.dumps(value)
else:
    value = value.encode('utf-8')

ou peut-être que j'ajouterai à la définition d'un attribut SAML la sérialisation qu'on souhaite.

#4

Mis à jour par Benjamin Dauvergne il y a plus de 6 ans

  • Assigné à mis à Benjamin Renard
#5

Mis à jour par Benjamin Renard il y a plus de 6 ans

Première implémentation fonctionnelle.

#6

Mis à jour par Benjamin Dauvergne il y a plus de 6 ans

  • découper en deux/trois méthode pour éviter que le code soit aussi profond, il faut séparer le code de récupération habituel des attributs de ce nouveau code
  • rendre serialization optionnel (i.e. par défaut None, pas 'json'), par défaut on conserve simplement la structure python sans sérialiser
  • un simple log.warning() pour l'erreur LDAP, pas besoin de trace dans ce cas, par contre on peut fournir le détail de la recherche LDAP tentée

J'aurai préféré que la sérialisation soit géré dans le code SAML mais en l'état c'est acceptable si la sérialisation est optionnelle.

#7

Mis à jour par Benjamin Renard il y a plus de 6 ans

Une nouvelle version du patch en suivant tes recommandations. Dit-moi si ça convient mieux ?

#8

Mis à jour par Benjamin Renard il y a plus de 6 ans

Benjamin Renard a écrit :

Une nouvelle version du patch en suivant tes recommandations. Dit-moi si ça convient mieux ?

Bonjour Benjamin, as-tu put regarder mon dernier patch ? Son intégration est-elle envisageable à court/moyen terme ?

#9

Mis à jour par Benjamin Renard il y a plus de 6 ans

Benjamin Renard a écrit :

Benjamin Renard a écrit :

Une nouvelle version du patch en suivant tes recommandations. Dit-moi si ça convient mieux ?

Bonjour Benjamin, as-tu put regarder mon dernier patch ? Son intégration est-elle envisageable à court/moyen terme ?

J'ai refait des tests ce jour sur un déploiement et j'ai corrigé un bug sur la prise en compte du mapping des attributs avec des majuscules après normalized. Je vous envoi un nouveau patch dans l'après midi.

#10

Mis à jour par Benjamin Renard il y a plus de 6 ans

Benjamin Renard a écrit :

J'ai refait des tests ce jour sur un déploiement et j'ai corrigé un bug sur la prise en compte du mapping des attributs avec des majuscules après normalized. Je vous envoi un nouveau patch dans l'après midi.

Ci-joint le patch incluant la correction de ce problème.

#11

Mis à jour par Benjamin Dauvergne il y a plus de 6 ans

La fonctionnalité est ok mais:
  • il faudrait un test dans tests/test_ldap.py
  • des lignes pas PEP8 (espace avant et après les symboles, ici =):
                    extra_attribute_config['loop_over_attribute']=str.lower(extra_attribute_config['loop_over_attribute'])
    
  • on préfèrera extra_attribute_config['loop_over_attribute'].lower() que str.lower(extra_attribute_config['loop_over_attribute'])
#12

Mis à jour par Paul Marillonnet il y a plus de 6 ans

Discuté aujourd'hui avec Benjamin R. de la façon dont on pourrait tester ça.

C'est vrai qu'un test unitaire pour le rapatriement d'attributs standards (paramètre 'attributes') serait utile pour ensuite écrire celui des 'extra_attributes'.

Peut-être quelque chose comme ça du coup ? (patch WIP joint)

#13

Mis à jour par Benjamin Dauvergne il y a plus de 6 ans

Oui c'est ok, Benjamin tu peux reprendre le test de Paul pour faire le tien.

#14

Mis à jour par Benjamin Renard il y a plus de 6 ans

Benjamin Dauvergne a écrit :

Oui c'est ok, Benjamin tu peux reprendre le test de Paul pour faire le tien.

Ci-joint mon patch mis à jours en incluant mon test et celui de Paul ainsi qu'un autre patch incluant la correction des erreurs pep8 présent dans les fichiers que j'ai modifié avant que j'y touche.

#15

Mis à jour par Benjamin Dauvergne il y a plus de 6 ans

Ça m'a l'air ok, je pousserai tout ça vendredi après notre cycle de release en cours.

#16

Mis à jour par Benjamin Renard il y a environ 6 ans

Benjamin Dauvergne a écrit :

Ça m'a l'air ok, je pousserai tout ça vendredi après notre cycle de release en cours.

Sauf erreur, j'ai pas l'impression que ça a été poussé sur le repos. Tu serai me dire pour quand c'est prévu ?

#17

Mis à jour par Benjamin Dauvergne il y a presque 6 ans

Patchs rebasés, je dois encore repasser dessus pour corriger le formatage et le style.

#18

Mis à jour par Benjamin Dauvergne il y a plus de 5 ans

  • Statut changé de Nouveau à En cours
  • Assigné à changé de Benjamin Renard à Benjamin Dauvergne
#19

Mis à jour par Paul Marillonnet il y a environ 4 ans

  • Statut changé de En cours à Information nécessaire

Ce serait quand même bien d'avoir ça, et le gros du travail est fait. Est-ce que tu veux que je m'occupe du rebasage et des corrections ?

#20

Mis à jour par Benjamin Dauvergne il y a environ 4 ans

Si tu veux oui.

#21

Mis à jour par Paul Marillonnet il y a environ 4 ans

  • Statut changé de Information nécessaire à En cours
  • Assigné à changé de Benjamin Dauvergne à Paul Marillonnet
#22

Mis à jour par Paul Marillonnet il y a presque 4 ans

#23

Mis à jour par Paul Marillonnet il y a presque 4 ans

NB : Benj (D.) dans #24022 :

ajouter un header License: MIT en bas de tes messages de commit

Je pense que ça s'applique aussi ici.

#24

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

  • Assigné à changé de Paul Marillonnet à Benjamin Dauvergne
#26

Mis à jour par Benjamin Renard il y a plus de 3 ans

Benjamin Dauvergne a écrit :

Rebasé, adapté pour py3.

Pour info, je viens de mettre à jour en py3 l'installation du client pour lequel j'avais implémenté cela. Suite à l'upgrade, j'ai appliqué ce patch à la place de celui basé sur py2 que j'utilisais jusqu'ici et tous fonctionne sans souci.

Pour quand l'intégration upstream ? :)

#27

Mis à jour par Paul Marillonnet il y a plus de 3 ans

  • Statut changé de Solution proposée à Solution validée

Rebasé sur master dans la branche wip/19365-manage-extra-ldap-attrs, c'est ok pour moi.

#28

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

  • Statut changé de Solution validée à Résolu (à déployer)
commit 7ac1fbcb4b31a980df1919340ba54e77c44e86cb
Author: Benjamin Renard <brenard@easter-eggs.com>
Date:   Mon Dec 11 19:56:26 2017 +0100

    Manage LDAP extra attributes (#19365)

    This extra attributes are retreived by making other LDAP queries
    with parameters composed by looping on an user object's attribute
    values. A mapping will be apply on corresponding objects's attributes
    and resulting informations are compiled in a list and serialize in
    JSON (configurable, but JSON is the only format available for now)
#29

Mis à jour par Frédéric Péters il y a plus de 3 ans

  • Statut changé de Résolu (à déployer) à Solution déployée

Formats disponibles : Atom PDF