Projet

Général

Profil

Development #40508

python3: artifacts vus comme du text

Ajouté par Nicolas Roche il y a environ 4 ans. Mis à jour il y a environ 4 ans.

Statut:
Fermé
Priorité:
Normal
Assigné à:
Benjamin Dauvergne
Catégorie:
-
Version cible:
-
Début:
06 mars 2020
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Oui
Planning:
Non

Description

Problème au login :

    response = return_login_response(request, login)
  File "/home/nroche/src/authentic/src/authentic2/idp/saml/saml2_endpoints.py", line 841, in return_login_response
    save_artifact(request, login)
  File "/home/nroche/src/authentic/src/authentic2/idp/saml/saml2_endpoints.py", line 865, in save_artifact
    content=login.artifactMessage.decode('utf-8'),
AttributeError: 'str' object has no attribute 'decode'

Fichiers

0001-python3-handle-artifact-message-as-text-40508.patch (2,56 ko) 0001-python3-handle-artifact-message-as-text-40508.patch Nicolas Roche, 06 mars 2020 14:45
0001-python3-use-Django-encoding-module-in-SAML-IdP-40508.patch (11,2 ko) 0001-python3-use-Django-encoding-module-in-SAML-IdP-40508.patch Benjamin Dauvergne, 06 mars 2020 16:00
0001-python3-use-Django-encoding-module-in-SAML-IdP-40508.patch (57,6 ko) 0001-python3-use-Django-encoding-module-in-SAML-IdP-40508.patch Benjamin Dauvergne, 10 mars 2020 19:49

Demandes liées

Lié à Publik Installation Développeur - Development #40403: Tourner authentic2 en python3Fermé04 mars 2020

Actions
Bloque Authentic 2 - Development #40722: idp_saml2: le relaystate n'est pas conservé si on reçoit une requête de SSO en POSTFermé13 mars 2020

Actions

Révisions associées

Révision 6df0289f (diff)
Ajouté par Nicolas Roche il y a environ 4 ans

python3: use Django encoding module in SAML IdP (#40508)

Historique

#1

Mis à jour par Nicolas Roche il y a environ 4 ans

#2

Mis à jour par Frédéric Péters il y a environ 4 ans

Vu que ça n'a pas été détecté par les tests, il faudrait certainement en ajouter. (parce que là zéro confiance que ça passe en Python 2).

#3

Mis à jour par Paul Marillonnet il y a environ 4 ans

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

(parce que là zéro confiance que ça passe en Python 2).

Aïe oui ça va casser la compat python2.

#4

Mis à jour par Nicolas Roche il y a environ 4 ans

#5

Mis à jour par Benjamin Dauvergne il y a environ 4 ans

Avec lasso il faut utiliser force_str, pas force_text (seul la version Python3 est "compatible" unicode).

#6

Mis à jour par Benjamin Dauvergne il y a environ 4 ans

Benjamin Dauvergne a écrit :

Avec lasso il faut utiliser force_str, pas force_text (seul la version Python3 est "compatible" unicode).

Pour être plus clair il faut utiliser :
  • dans le sens a2 vers Lasso : force_str
  • dans le sens Lasso vers a2 : force_text
#7

Mis à jour par Benjamin Dauvergne il y a environ 4 ans

J'ai repris toutes les modifications faites par Paul pour y appliquer cette règle, reste à augmenter la couverture des tests.

#8

Mis à jour par Nicolas Roche il y a environ 4 ans

tl;dr (en essayant d'écrire le test)
Dans authentic2/idp/saml/saml2_endpoints.py,
pour couvrir save_artifact, je n'arrive pas à touver comment passer ici :

if login.protocolProfile == lasso.LOGIN_PROTOCOL_PROFILE_BRWS_ART:

Dans lasso, Je vois bien saml2_strings.h :

#define LASSO_SAML2_METADATA_BINDING_ARTIFACT "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Artifact"

et login.c :
if (lasso_strisequal(protocol_binding,LASSO_SAML2_METADATA_BINDING_ARTIFACT)) {
    login->protocolProfile = LASSO_LOGIN_PROTOCOL_PROFILE_BRWS_ART;

Ainsi que les métadonnées de l'IDP du test tests/test_idp_saml2.py qui y font référence :

   <AssertionConsumerService
     index="1" 
     Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Artifact" 
     Location="{base_url}/mellon/artifactResponse" />
   </SPSSODescriptor>

Mais je ne fais pas le lien avec debian/debian_config_common.py :

MELLON_DEFAULT_ASSERTION_CONSUMER_BINDING = 'artifact'

ni avec les métadonnée que j'ai en local (installation via public-devinst) dans /var/lib/combo/tenants/combo.dev.publik.love/idp-metadata-1.xml :
...
ArtifactResolutionService Binding="urn:oasis:names:tc:SAML:2.0:bindings:SOAP"

J'aurais bien dit que le lien est fait par idp_sso mais je ne trouve pas qui l'appelle.

#9

Mis à jour par Benjamin Dauvergne il y a environ 4 ans

  • Assigné à mis à Benjamin Dauvergne
#10

Mis à jour par Benjamin Dauvergne il y a environ 4 ans

Il ne faut pas regarder du coté de MELLON_* sauf si tu montes tes tests avec mellon, mais je vais m'en occuper.

#11

Mis à jour par Benjamin Dauvergne il y a environ 4 ans

Voilà save_artifact est couvert, il manque encore pas mal de couverture mais pour ce point c'est bon en python2 et 3.

#12

Mis à jour par Nicolas Roche il y a environ 4 ans

Ça se passe donc dans les les métadonnées de l'IDP (via la première balise AssertionConsumerService trouvée).
Si je comprenais quelque-chose à la GI je validerais : les tests me semblent beaucoup plus accessibles.

#13

Mis à jour par Paul Marillonnet il y a environ 4 ans

  • Statut changé de Solution proposée à Solution validée

Nicolas Roche a écrit :

[…] les tests me semblent beaucoup plus accessibles.

Yep je préfère comme ça aussi. Du code churn mais les tests sont plus lisibles.

Quelques suggestions pour la forme, sans grande importance :

diff --git a/src/authentic2/idp/saml/saml2_endpoints.py b/src/authentic2/idp/saml/saml2_endpoints.py
index 71be1e05c..e209c62b3 100644
--- a/src/authentic2/idp/saml/saml2_endpoints.py
+++ b/src/authentic2/idp/saml/saml2_endpoints.py
@@ -1,5 +1,5 @@
 # authentic2 - versatile identity manager
-# Copyright (C) 2010-2019 Entr'ouvert
+# Copyright (C) 2010-2020 Entr'ouvert
 #
 # This program is free software: you can redistribute it and/or modify it
 # under the terms of the GNU Affero General Public License as published
diff --git a/tests/test_idp_saml2.py b/tests/test_idp_saml2.py
index 9cf478bb0..dfe6c528e 100644
--- a/tests/test_idp_saml2.py
+++ b/tests/test_idp_saml2.py
@@ -68,8 +68,6 @@ def keys():
     return (cert, key)

-
-
 @pytest.fixture(autouse=True)
 def idp(saml_settings, db):
     code_attribute = Attribute.objects.create(kind='string', name='code', label='Code')
@@ -105,7 +103,7 @@ def user(idp):
 class SamlSP(object):
     METADATA_TPL = '''<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
 <EntityDescriptor
- entityID="{{base_url}}/" 
+ entityID="{{ base_url }}/" 
  xmlns="urn:oasis:names:tc:SAML:2.0:metadata">
  <SPSSODescriptor
    AuthnRequestsSigned="true" 
@@ -126,12 +124,12 @@ class SamlSP(object):
      index="0" 
      isDefault="true" 
      Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" 
-     Location="{{base_url}}/sso/POST" />
+     Location="{{ base_url }}/sso/POST" />
    {% elif binding == 'artifact' %}
    <AssertionConsumerService
      index="0" 
      Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Artifact" 
-     Location="{{base_url}}/mellon/artifactResponse" />
+     Location="{{ base_url }}/mellon/artifactResponse" />
    {% endif %}
  </SPSSODescriptor>
 </EntityDescriptor>'''
diff --git a/tests/utils.py b/tests/utils.py
index ae16af202..f95f47d95 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -1,5 +1,5 @@
 # authentic2 - versatile identity manager
-# Copyright (C) 2010-2019 Entr'ouvert
+# Copyright (C) 2010-2020 Entr'ouvert
 #
 # This program is free software: you can redistribute it and/or modify it
 # under the terms of the GNU Affero General Public License as published

Sinon c'est bon pour moi, ack.

#14

Mis à jour par Benjamin Dauvergne il y a environ 4 ans

  • Bloque Development #40722: idp_saml2: le relaystate n'est pas conservé si on reçoit une requête de SSO en POST ajouté
#15

Mis à jour par Benjamin Dauvergne il y a environ 4 ans

  • Statut changé de Solution validée à Résolu (à déployer)
commit 6df0289f6e69f128b9eae7c6e5ab0d95cf1e7d50
Author: Nicolas ROCHE <nroche@entrouvert.com>
Date:   Fri Mar 6 14:42:55 2020 +0100

    python3: use Django encoding module in SAML IdP (#40508)
#16

Mis à jour par Frédéric Péters il y a environ 4 ans

  • Statut changé de Résolu (à déployer) à Solution déployée

Formats disponibles : Atom PDF