Produits Entr'ouvert » django-mellon

Je me pose la question de l'utilité, souvent on a à débugguer un accès par un tiers, pas son propre compte admin. Dernier exemple en date, #55353#note-3, où on reste à devoir deviner "Il doit manquer une des clés qui sont indiqués comme mandatory", et ce patch n'aidera pas.

On ne pourrait pas imaginer, authentic, journaliser toutes les assertions (valides/signées) reçues ?

Ainsi on aurait au max à demander à quelle heure la tentative, filtrer le journal vers ce moment, et lire l'assertion.

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

Je me pose la question de l'utilité, souvent on a à débugguer un accès par un tiers, pas son propre compte admin. Dernier exemple en date, #55353#note-3, où on reste à devoir deviner "Il doit manquer une des clés qui sont indiqués comme mandatory", et ce patch n'aidera pas.

Il y a déjà un message qui s'affiche dans ce cas; là il n'est pas là, donc ce n'est peut-être pas le problème. L'assertion aurait été poussé dans les logs que ce serait pareil, vu qu'on a plus les logs.

On ne pourrait pas imaginer, authentic, journaliser toutes les assertions (valides/signées) reçues ?

Oui, mais avec des logs de deux semaines, et deux semaines pour que quelqu'un regarde dans les logs ça n'aidera pas.

Quand j'écris "journaliser" je pensais au journal, /manage/journal/, il n'y a pas cette expiration à deux semaines là.

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

Quand j'écris "journaliser" je pensais au journal, /manage/journal/, il n'y a pas cette expiration à deux semaines là.

Alors là ça devient un autre ticket, il faudrait un logging.Handler à poser sur les domaines authentic2_auth_saml et mellon qui enverrait les messages de auth_saml/mellon dans le journal, je suppose qu'il faudrait aussi ajouter au journal la possibilité d'afficher des popups dans le style des logs passerelle pour les objets un peu gros ne rentrant pas sur une ligne (style un message SAML). Ceci pour éviter d'avoir douzes façons de logger dans le code.

Benjamin Dauvergne a écrit :

Alors là ça devient un autre ticket

Moi je pensais que c'était exclu parce que ce message SAML est gros : encodé en ASCII chez moi ça fait 6Ko, si on se dit qu'on a 5000 connexions par jour par SAML, ça fait 1Go de données par mois. Voilà j'ai aucune idée de si c'est signifiant ou si c'est rien du tout.

il faudrait un logging.Handler à poser sur les domaines authentic2_auth_saml et mellon qui enverrait les messages de auth_saml/mellon dans le journal

En moins compliqué mais peut-être plus schlag, request.session['mellon_saml_response'] = saml_response et libre à qui utilise mellon d'accéder à ça et d'en faire ce qu'il veut ? C'est deux patches d'une ligne pour arriver à ce qu'on veut.

je suppose qu'il faudrait aussi ajouter au journal la possibilité d'afficher des popups dans le style des logs passerelle pour les objets un peu gros ne rentrant pas sur une ligne (style un message SAML)

Ça oui mais ça me paraît une bonne évolution.

Valentin Deniaud a écrit :

Benjamin Dauvergne a écrit :

Alors là ça devient un autre ticket

Moi je pensais que c'était exclu parce que ce message SAML est gros : encodé en ASCII chez moi ça fait 6Ko, si on se dit qu'on a 5000 connexions par jour par SAML, ça fait 1Go de données par mois. Voilà j'ai aucune idée de si c'est signifiant ou si c'est rien du tout.

Je n'avais pas fait le calcul en tête, mais oui ça doit être ça, c'est pour ça que je suggérai une vue de debug parce que le cas nominal ce sont les soucis au premier raccordement, où un développeur/CPT est en train de faire le raccordement et donc peut utiliser cette vue de debug.

Le cas lié ici d'une personne pour laquelle ça ne marche pas, alors que ça marche pour les autres, ça devrait être réglé par des logs précis que normalement on a déjà, la résolution du ticket nous éclairera sur ce point. Je ne suis pas certain que dumper l'assertion nous aide plus.

il faudrait un logging.Handler à poser sur les domaines authentic2_auth_saml et mellon qui enverrait les messages de auth_saml/mellon dans le journal

En moins compliqué mais peut-être plus schlag, request.session['mellon_saml_response'] = saml_response et libre à qui utilise mellon d'accéder à ça et d'en faire ce qu'il veut ? C'est deux patches d'une ligne pour arriver à ce qu'on veut.

Oui aussi.

je suppose qu'il faudrait aussi ajouter au journal la possibilité d'afficher des popups dans le style des logs passerelle pour les objets un peu gros ne rentrant pas sur une ligne (style un message SAML)

Ça oui mais ça me paraît une bonne évolution.

Oui mais avant de se disperser je me dis qu'il faut comprendre pourquoi dans le cas lié on a pas le message d'erreur en front avec l'erreur de mapping qu'on aurait du avoir si c'est vraiment une erreur de mapping (manque un attribut) qui a bloqué le SSO.

Je reviens au patch, plutôt que d'utiliser lasso_node_dump() utiliser lasso_node_debug(), via response_dump = login.response and login.response.debug(4) (4 c'est l'indentation), je dumperai login.assertion aussi (c'est dans la réponse, mais c'est plus court à analyser).

• attributs
• assertion
• response
• artifact/POST (parce que msgBody ça peut aussi être le contenu du POST en cas de binding POST, en fait ce sera la même chose que dans login.response mais encodé).

Est-ce qu'on ne laisserait pas la vue active pour tout le monde aussi si DEBUG = True ?

Et si le cas DEBUG = True est implémenté: j'ai l'impression qu'une fois mellon_debug_login posé il ne se ré-initialise jamais, il faudrait le virer dans DebugLoginView.authenticate, et ajouter un lien "Try Again" au template (<a class="button" href="?">Try again</a>).

Et dernière remarque : je laissera l'authentification se faire (via django.contrib.auth.authenticate()), juste je ne loggerai pas l'utilisateur (on shunte l'appel à utils.login(...)), et j'afficherai l'utilisateur qui serait authentifié ainsi que tous les logs entre temps (via https://docs.python.org/3/library/logging.handlers.html#memoryhandler, posé sur le logger mellon et authentic2_auth_saml temporairement). Comme ça on sait tout.

PS: aussi exemple, https://docs.python.org/3/howto/logging-cookbook.html#buffering-logging-messages-and-outputting-them-conditionally

Selon moi "if not request.user.is_superuser" est en trop, ça va compliquer le debug quand on n'aura pas un accès à l'idp (pas de compte test ou pas d'accès à l'idp du tout : mon idée est est dans ce cas on inviterait le client à tester par lui même et nous envoyer la page de résultat en copier-coller).

Aussi, y'a un self.log.root.addHandler, mais pas de suppression ensuite : est-ce qu'on ne risque pas ici d'activer un log permanent par la suite ? (Si c'est trop compliqué, je pense qu'on peut retirer les logs dans un premier temps, histoire d'avoir au moins un debug SAML de base)

Valentin Deniaud:

Je pense qu'on a pas le droit de faire référence à authentic2_auth_saml mais poser le handler sur le root logger ça fait ce qu'on veut j'ai l'impression. Aussi pas compris l'histoire de MemoryHandler, j'ai fait sans.

Parce qu'à la lecture de la doc de logging je me suis laissé à penser qu'il fallait faire absolument comme ça, mais ton StreamHandler sur un io.StreamIO semble effectivement encore plus simple et naturel.

Thomas Noël a écrit :

Selon moi "if not request.user.is_superuser" est en trop, ça va compliquer le debug quand on n'aura pas un accès à l'idp (pas de compte test ou pas d'accès à l'idp du tout : mon idée est est dans ce cas on inviterait le client à tester par lui même et nous envoyer la page de résultat en copier-coller).

Ça me gêne vraiment que ce soit ouvert tout le temps (les audits à la con tout ça), mais si superuser est jugé trop dur, on peut se baser sur settings.DEBUG; pour l'utiliser en prod pour nous ou un client il suffit de mettre le ou les IPs concernée dans les IPs de debug dans hobo (/debug/) sur hobo.

Aussi, y'a un self.log.root.addHandler, mais pas de suppression ensuite : est-ce qu'on ne risque pas ici d'activer un log permanent par la suite ? (Si c'est trop compliqué, je pense qu'on peut retirer les logs dans un premier temps, histoire d'avoir au moins un debug SAML de base)

Oui pareil pour addHandler j'ai fait une remarque équivalente sur un addFilter sur un ticket passerelle, faut faire attention quand on touche aux loggers, sinon on laisse du bordel derrière, j'en ferai un contextmanager comme je l'ai dit dans l'autre ticket, c'est le code pythonesque normal pour allouer/désallouer une ressource.

Benjamin Dauvergne a écrit :

Thomas Noël a écrit :

Selon moi "if not request.user.is_superuser" est en trop, ça va compliquer le debug quand on n'aura pas un accès à l'idp (pas de compte test ou pas d'accès à l'idp du tout : mon idée est est dans ce cas on inviterait le client à tester par lui même et nous envoyer la page de résultat en copier-coller).

Ça me gêne vraiment que ce soit ouvert tout le temps (les audits à la con tout ça), mais si superuser est jugé trop dur, on peut se baser sur settings.DEBUG; pour l'utiliser en prod pour nous ou un client il suffit de mettre le ou les IPs concernée dans les IPs de debug dans hobo (/debug/) sur hobo.

Ouaip mais bon, DEBUG=True ça pourrait suffire, selon moi (ça s'expliquer lors d'un audit)... Dans le wip proposé c'est « if not request.user.is_superuser or not settings.DEBUG: » et je proposais de remplacer par « if not settings.DEBUG: »...

Ceci dit, ça ferme le debug en prod (on n'y fait jamais DEBUG=True), et donc, pourquoi pas : « if not (settings.DEBUG or request.user.is_superuser): » ?

Thomas Noël a écrit :

Ceci dit, ça ferme le debug en prod (on n'y fait jamais DEBUG=True), et donc, pourquoi pas : « if not (settings.DEBUG or request.user.is_superuser): » ?

On peut toujours activer le DEBUG en prod via https://hobo.client.fr/debug/ pour des adresses IPs données.