Projet

Général

Profil

Development #3854

Utiliser la nouvelle méthode d'extractions des attributs SAML 2 issue de w.c.s.

Ajouté par Benjamin Dauvergne il y a plus de 10 ans. Mis à jour il y a plus de 10 ans.

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

100%

Temps estimé:
Patch proposed:
Planning:

Fichiers

Révisions associées

Révision e80f903b (diff)
Ajouté par Benjamin Dauvergne il y a plus de 10 ans

saml2: use new w.c.s method for filling user fields from assertion attributes

Old method is kept as legacy and is only active if the new method is not
configured.

fixes #3854

Historique

#1

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

  • Fichier 0001-saml2-use-new-w.c.s-method-for-filling-user-fields-f.patch ajouté
#2

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

  • Fichier 0001-saml2-use-new-w.c.s-method-for-filling-user-fields-f.patch supprimé
#3

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

  • Fichier 0001-saml2-use-new-w.c.s-method-for-filling-user-fields-f.patch ajouté
#4

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

  • Fichier 0001-saml2-use-new-w.c.s-method-for-filling-user-fields-f.patch supprimé
#6

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

formdata = user.form_data = user.form_data or {}

Détail stylistique, je ne trouve pas cette forme très pythonique.

self.fill_user_attributes(session, login, user)
self.legacy_fill_user_attributes(session, login, user)

J'aurais plutôt placé le legacy_ avant, et que celui-ci ne soit pas sans effet si un paramétrage a été effectué. Ou alors, si on trouve ce comportement meilleur, je préférerais le test (not idp.get('attribute-mapping')) dans le lookup_user.

extract roles from assertion attribute named 'role':

La suppression de la partie "rôles" pourrait se faire dans un commit distinct.

legacy_fill_is_admin

Il me semblait pourtant que le paramétrage de l'admin sur présence d'un attribut is_admin était placé comme comportement par défaut dans wcs, pourquoi faut-il le garder ici ?

#7

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

Frédéric Péters a écrit :

formdata = user.form_data = user.form_data or {}

Détail stylistique, je ne trouve pas cette forme très pythonique.

J'ai modifié fill_user_attributes() dans wcs pour qu'il s'assure que user.form_data existe, c'est donc devenu inutile.

self.fill_user_attributes(session, login, user)
self.legacy_fill_user_attributes(session, login, user)

J'aurais plutôt placé le legacy_ avant, et que celui-ci ne soit pas sans effet si un paramétrage a été effectué. Ou alors, si on trouve ce comportement meilleur, je préférerais le test (not idp.get('attribute-mapping')) dans le lookup_user.

Ok test déplacé, je préfère que la méthode legacy qui est "magique" n'impacte pas la méthode explicite.

extract roles from assertion attribute named 'role':

La suppression de la partie "rôles" pourrait se faire dans un commit distinct.

Ok.

legacy_fill_is_admin

Il me semblait pourtant que le paramétrage de l'admin sur présence d'un attribut is_admin était placé comme comportement par défaut dans wcs, pourquoi faut-il le garder ici ?

Oui, j'ai viré ce code.

#8

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

Je pense avoir répondu aux principales objections, je vais pousser.

#9

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

  • Statut changé de Nouveau à Résolu (à déployer)
  • % réalisé changé de 0 à 100
#10

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

Tu peux lister les objections auxquelles tu n'as pas répondu ? :)

#11

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

  • Statut changé de Résolu (à déployer) à Fermé

Formats disponibles : Atom PDF