Produits Entr'ouvert » Authentic 2

Je suis contre ce ticket, c'est soit on fait les choses bien, et donc stocker avec le reste de la configuration OIDC, soit on reste sur le hook pour l'instant.

Benjamin Dauvergne a écrit :

Je suis contre ce ticket, c'est soit on fait les choses bien, et donc stocker avec le reste de la configuration OIDC

Ok, par config oidc tu entends config du client ?

Oui, un modèle du genre:

class OIDCAttribute
   client = FK(OIDCClient)
   name = CharField() # "adress.street" <- sera déplié
   value = CharField() # le nom d'un attribut comme coté SAML
   scope = CharField() # une liste de scopes dont il fait partie

idéalement ça devrait gérer le cas où un attribut est vérifié en servant le même nom avec le suffixe _verified comme le prévoit la norme.

Une migration de donnée devrait ajouter les attributs existants comme OIDCAttribute et on pourrait ensuite supprimer le code en dur.

Oui le code est un peu brut:

def get_attributes(instance, ctx):
    user = ctx.get('user')
    User = get_user_model()
    if not user or not isinstance(user, User):
        return ctx
    for field in User._meta.fields:
        value = getattr(user, field.name)
        if value is None:
            continue
        ctx['django_user_' + str(field.name)] = getattr(user, field.name) # pour django_user_ou on va obtenir le modèle directement
    for av in AttributeValue.objects.with_owner(user):
        ctx['django_user_' + str(av.attribute.name)] = av.to_python()
        ctx['django_user_' + str(av.attribute.name) + ':verified'] = av.verified
    ctx['django_user_groups'] = [group for group in user.groups.all()]
    ctx['django_user_group_names'] = [unicode(group) for group in user.groups.all()]
    if user.username:
        splitted = user.username.rsplit('@', 1)
        ctx['django_user_domain'] = splitted[1] if '@' in user.username else ''
        ctx['django_user_identifier'] = splitted[0] if '@' in user.username else ''
    ctx['django_user_full_name'] = user.get_full_name()
    Role = get_role_model()
    roles = Role.objects.for_user(user)
    ctx['a2_role_slugs'] = roles.values_list('slug', flat=True)
    ctx['a2_role_names'] = roles.values_list('name', flat=True)
    ctx['a2_role_uuids'] = roles.values_list('uuid', flat=True)
    if 'service' in ctx and getattr(ctx['service'], 'ou', None):
        ou = ctx['service'].ou
        ctx['a2_service_ou_role_slugs'] = roles.filter(ou=ou).values_list('slug', flat=True)
        ctx['a2_service_ou_role_names'] = roles.filter(ou=ou).values_list('name', flat=True) <-- ValuesQuerySet
        ctx['a2_service_ou_role_uuids'] = roles.filter(ou=ou).values_list('uuid', flat=True)
    return ctx

Coté SAML je suppose que ça applique unicode() là dessus ou que ça génère plusieurs valeurs si ça détecte une liste, et oui c'est ça (ctx est le résultat de l'appel à get_attributes()).

    tuples = [tuple(t) for definition in qs for t in definition.to_tuples(ctx)]
    seen = set()
    for name, name_format, friendly_name, value in tuples:

    def to_tuples(self, ctx):
        if not self.attribute_name in ctx:
            return
        name_format = self.name_format_uri()
        name = self.name
        friendly_name = self.friendly_name or None
        values = ctx.get(self.attribute_name)
        for text_value in normalize_attribute_values(values):
            yield (name, name_format, friendly_name, text_value)

def normalize_attribute_values(values):
    '''Take a list of values or a single one and normalize it'''
    values_set = set()
    if isinstance(values, basestring) or not hasattr(values, '__iter__'):
        values = [values]
    for value in values:
        if isinstance(value, bool):
            value = str(value).lower()
        values_set.add(unicode(value))
    return values_set

• en OIDC on appelle les attributs des "claim"s, renomme OIDCAttribut en OIDCClaim pour rester dans le jargon OIDC

• modifie normalize_attribute_values() pour ne pas systématiquement convertir les valeurs uniques en liste (c'est normal en SAML mais en OIDC qui fait la différence entre liste et valeur unique puisque le format de transfert c'est du JSON, ce n'est pas nécessaire), tu peux en faire un flag ou mieux deux fonctions différentes, idem sur la normalisation vers unicode, je ne suis pas sûr que ce soit toujours nécessaire, il vaut peut-être mieux le réserver pour les modèles, en fait il faut peut-être mieux un normalize_attribute_values() spécifique à OIDC

• applique simplement normalize_attribute_values() à tes valeurs avant d'utiliser la valeur dans user_info

• manque la migration de donnée pour créer les OIDCClaim pour la configuration de base

• dans l'admin si on ne définit pas d'attributs (il y a un InlineFormSet, on doit pouvoir détecter qu'il est vide, et/ou l'initialiser dans la vue de création d'un OIDCClient) créer aussi la configuration de base

• j'aurai plutôt vu l'association scope<->claim sur un objet OIDCScope mais restons comme cela pour l'instant
• d'après la spécification OIDC on peut demander un claim via son scope mais aussi directement par son nom (chaque nom de claim est lui même un scope normalement)

Tu ne devrais pas avoir à poser oidcclient explicitement dans une ModelForm, l'objet en cours est disponible dans self.instance.

L'initialisation de initial peut se faire même en dehors d'un POST et juste avant cette ligne:

        formset.__init__ = curry(formset.__init__, initial=initial)

Je te propose de définir un get_service_variables(service) à partager avec le code SAML équivalent dans authentic2/saml/admin.py.

Au passage un besoin de dernière minute, ce serait bien de regarder aussi la présence d'une clé claim.value + ':verified' dans attributes et si c'est présent et vrai de poser un claim claim.name + '_verified' dans user_info avec la valeur True. C'est la façon standard en OIDC de passer le statut vérifié d'un attribut.

Ajoute une commentaire au dessus de

# formsets are only saved if formset.has_changed() is True, so only set initial 
# values on the GET (display of the creation form)
if request.method == 'GET' and not obj:

et ack.