Produits Entr'ouvert » Authentic 2

Avant de coder il faudrait établir les champs exposés dans le formulaire d'ajout/édition, car tous les attributs visibles dans l'admin ne sont pas parlants/compréhensibles (même par les CPTs).
Et sur la base de ces champs faire une doc.

En plus des champs exposés déjà (nom, slug, collectivité, url de retour quand non-autorisé) je propose:

• URIs de redirection
• URIs de redirection après déconnexion
• URI de déconnexion frontchannel
• Politique des identifiants
• Algo de signature du token
• URL d’accueil
• Logo
• Couleur
• Mode d'autorisation
• Processus d'autorisation

Les claims ne seront pas exposés car on n'a presque jamais eu de demande de modification de ceux-ci.
Les client_id et client_secret seront affichés sur la page du service sans possibilité de les modifier.

Les claims ne seront pas exposés car on n'a presque jamais eu de demande de modification de ceux-ci.

Ça fait partie des demandes actuelles Strasbourg et sur les situations où il faut envoyer un gender qu'on base sur le title, ça veut dire qu'on devra quand même aller dans l'admin; ok pour que ça ne soit pas présent dans ce ticket mais avis perso ça devra venir à un moment.

De la première capture je ne pense pas qu'on ait ailleurs de présentation en tableau, on privilégie soit libellé: valeur, soit libellé retour à la ligne valeur.

De la deuxième capture d'écran je dirais qu'il ne faut pas ouvrir en popup la création (ou la limiter aux champs strictement nécessaires).

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

De la deuxième capture d'écran je dirais qu'il ne faut pas ouvrir en popup la création (ou la limiter aux champs strictement nécessaires).

Oui pas de popup, pour les objets complexe qu'on édite peu ce n'est vraiment pas pratique.

Après un premier essai rapide en local :
· cela reste une popup lors de la modification du service (pas sûr que ce soit le comportement souhaité ?)

· une erreur de validation à la modification décale la popup seulement sur la partie droit de l’écran (cf capture).

· une première erreur de validation, ensuite corrigée, bloque définitivement la popup. Il n’y a plus moyen de valider le formulaire une fois l’erreur corrigée.

· on n’a perdu la fonctionnalité de complétion de la valeur des claims telle que proposée dans le /admin/, je pense que c’est utile de conserver ça.

· il n’y a nulle part où voir quels ont été les client_id et client_secret générés à la création du client. Je pense qu’il faut les faire apparaître en lecture seule.

· on s’attendrait à pouvoir éditer aussi la liste des “rôles visibles uniquement de ce service”, mais ce n’est pas le cas. Est-ce qu’il y a une raison à laisser en édition la liste des “rôles autorisés à se connecter à ce service” mais pas celle des “rôles visibles uniquement de ce service” ?

Une première lecture en diagonale du code, je vois un

+    def get_object(self, queryset=None):
+        service = super().get_object(queryset)
+        if hasattr(service, 'oidcclient'):
+            self.model = OIDCClient
+            return service.oidcclient
+        return service

Ok, merci pour tes explications, il me manquait deux trois billes pour comprendre le truc.

Tout relu, le code est clair mais le seul truc qui me gêne un peu c’est le {{ service_block|safe }}. Ça m’a l’air trop peu django-esque de rendre un si gros bout de html directement dans la vue puis de le passer dans le gabarit final avec ce filtre safe ensuite.
Intuitivement, j’aurais bien vu un {% include extra_details %} avec cette valeur extra_details initialisée dans la vue à "authentic2/manager/oidc_service_detail.html", et en passant aussi le reste du contexte nécessaire au rendu de ce sous-gabarit (bien sûr pour le reste des services non OIDC ce extra_details n’est pas défini et donc cette balise include reste sans effet).

Autre chose encore et vraiment du détail, mais sur tous les titres "Add OpenID Claim", "Add OpenID service", etc. j’aurais bien vu un s/OpenID/OIDC/ pour éviter la confusion (OpenID est encore un autre protocole¹, ancêtre d’OIDC et pas du tout basé sur OAuth 2.0, et obsolète maintenant).

1. https://openid.net/specs/openid-authentication-2_0.html

Serghei Mihai a écrit :

J'ai suivi le même principe que les blocs de login et inscription qui font leur rendu HTML et l'incluent dans un template global. Mais c'est plus propre, en effet, d'inclure un template dédié.

Top.

Ok, done (jenkins devrait être contenu sous peu).

Merci.

J’ai testé et tout me va, il me reste une dernière interrogation (que je n’avais pas relevée à ma dernière relecture, mes excuses), pourquoi ne pas utiliser le widget de choix de couleur natif du navigateur pour la couleur du client ? Ça a l’air supporté partout de façon systématique (sauf IE mais tant pis, un admin a2 sous IE ne pourra pas choisir la couleur du client OIDC, pas bien dramatique, non ?)

Il y aurait moyen de mettre les vues/templates/urls/form dans le module authentic2_idp_oidc ? Dans l'idée de ne pas charger le code générique, puisqu'il faudra ajouter SAML et CAS par la suite. Ça vaudrait aussi pour get_oidc_service_data qui pourrait être une méthode du modèle OIDCClient.

Pour rapprocher un peu le code de ce qui a été fait pour les moyens d'authentification je verrais bien OIDCClient qui définisse un manager_form_class, ce qui permettrait de s'économiser la gymnastique fields.extend(oidc_app_settings.MANAGER_FIELDS).

Valentin Deniaud a écrit :

Il y aurait moyen de mettre les vues/templates/urls/form dans le module authentic2_idp_oidc ? Dans l'idée de ne pas charger le code générique, puisqu'il faudra ajouter SAML et CAS par la suite.

CAS on pourra s’épargner cela (#34257, pas un seul raccordement encore actif chez nous à ma connaissance), et je ne sais pas dans quelle mesure on a intérêt à ajouter SAML au lieu de dire "OIDC everywhere" pour ce qui est des raccordements avec a2 en tant qu’IdP (?)

• self.model = OIDCClient et model = None, j'ai pas l'impression qu'il y ait d'usage de ça ?
• Est-ce qu'il y a une vraie utilité au setting MANAGER_FIELDS ? Je ne suis même pas sûr que le changer fonctionne sans redémarrage de l'application. À la place on pourrait avoir de manière plus classique la définition des champs dans le formulaire et ensuite accéder à form._meta.fields.

À part ça c'est OK pour moi (mais je laisse Paul valider, je n'ai pas testé pour de vrai).

Serghei Mihai a écrit :

C'est pour laisser la main de changer cette liste dans le settings d'un tenant, si besoin.
C'est censé de fonctionner sans redémarrage.

Du coup j'ai testé et ça ne fonctionne que pour les display_fields, les champs du formulaire restent les mêmes, la classe étant chargée une seule fois au démarrage de l'appli. Je propose de se passer de cette feature :)

Automatic expiration