Project

General

Profile

Development #19365

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

Added by Benjamin Renard over 2 years ago. Updated 2 days ago.

Status:
Solution proposée
Priority:
Normal
Category:
-
Target version:
-
Start date:
11 Oct 2017
Due date:
% Done:

0%

Patch proposed:
Yes
Planning:
No

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

0001-Manage-LDAP-extra-attributes-19365.patch View - Patch (6.12 KB) Benjamin Renard, 13 Oct 2017 02:06 PM

0001-Manage-LDAP-extra-attributes-19365.patch View (6.48 KB) Benjamin Renard, 17 Oct 2017 04:31 PM

0001-Manage-LDAP-extra-attributes-19365.patch View (6.49 KB) Benjamin Renard, 15 Nov 2017 01:56 PM

0001-WIP-ldap_backend-add-attribute-retrieval-unit-testin.patch View (1.92 KB) Paul Marillonnet, 08 Dec 2017 03:14 PM

0001-Manage-LDAP-extra-attributes-19365.patch View (11.5 KB) Benjamin Renard, 11 Dec 2017 07:57 PM

0002-Fix-somes-pep8-errors.patch View (3.11 KB) Benjamin Renard, 11 Dec 2017 07:57 PM

0001-Manage-LDAP-extra-attributes-19365.patch View (11.4 KB) Benjamin Dauvergne, 17 May 2018 03:54 PM

0002-Fix-somes-pep8-errors.patch View (3.11 KB) Benjamin Dauvergne, 17 May 2018 03:54 PM

0001-Manage-LDAP-extra-attributes-19365.patch View (10.7 KB) Paul Marillonnet, 31 Mar 2020 10:19 AM

History

#1 Updated by Benjamin Renard over 2 years ago

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 Updated by Benjamin Renard over 2 years ago

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 Updated by Benjamin Dauvergne over 2 years ago

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 Updated by Benjamin Dauvergne over 2 years ago

  • Assignee set to Benjamin Renard

#5 Updated by Benjamin Renard over 2 years ago

Première implémentation fonctionnelle.

#6 Updated by Benjamin Dauvergne over 2 years ago

  • 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 Updated by Benjamin Renard over 2 years ago

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

#8 Updated by Benjamin Renard over 2 years ago

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 Updated by Benjamin Renard over 2 years ago

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 Updated by Benjamin Renard over 2 years ago

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 Updated by Benjamin Dauvergne over 2 years ago

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 Updated by Paul Marillonnet over 2 years ago

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 Updated by Benjamin Dauvergne over 2 years ago

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

#14 Updated by Benjamin Renard over 2 years ago

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 Updated by Benjamin Dauvergne over 2 years ago

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

#16 Updated by Benjamin Renard about 2 years ago

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 Updated by Benjamin Dauvergne almost 2 years ago

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

#18 Updated by Benjamin Dauvergne over 1 year ago

  • Assignee changed from Benjamin Renard to Benjamin Dauvergne
  • Status changed from Nouveau to En cours

#19 Updated by Paul Marillonnet about 1 month ago

  • Status changed from En cours to 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 Updated by Benjamin Dauvergne about 1 month ago

Si tu veux oui.

#21 Updated by Paul Marillonnet about 1 month ago

  • Status changed from Information nécessaire to En cours
  • Assignee changed from Benjamin Dauvergne to Paul Marillonnet

#22 Updated by Paul Marillonnet 2 days ago

#23 Updated by Paul Marillonnet 2 days ago

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.

Also available in: Atom PDF