Project

General

Profile

Development #67090

saml: lier la session en cours à celle de l'IdP

Added by Benjamin Dauvergne 7 months ago. Updated 22 days ago.

Status:
Solution déployée
Priority:
Normal
Target version:
-
Start date:
06 July 2022
Due date:
% Done:

0%

Estimated time:
Patch proposed:
Yes
Planning:
No

Description

Comme #66747 et #67084 sur django-mellon.

PS: en plus le SSO passif ne marche plus du tout en fait.


Files

0003-tests-use-cookie-name-in-get_session-app-67090.patch (1.3 KB) 0003-tests-use-cookie-name-in-get_session-app-67090.patch Benjamin Dauvergne, 06 July 2022 07:19 PM
0001-misc-retry-passive-sso-when-session-expire-and-logou.patch (7.07 KB) 0001-misc-retry-passive-sso-when-session-expire-and-logou.patch Benjamin Dauvergne, 06 July 2022 07:19 PM
0002-saml2-store-current-value-of-OPENED_SESSION_COOKIE-i.patch (1.62 KB) 0002-saml2-store-current-value-of-OPENED_SESSION_COOKIE-i.patch Benjamin Dauvergne, 06 July 2022 07:19 PM
0005-test-modify-test-to-test-on-a-real-form-67090.patch (3.42 KB) 0005-test-modify-test-to-test-on-a-real-form-67090.patch Benjamin Dauvergne, 15 December 2022 05:41 PM
0004-tests-use-cookie-name-in-get_session-app-67090.patch (1.29 KB) 0004-tests-use-cookie-name-in-get_session-app-67090.patch Benjamin Dauvergne, 15 December 2022 05:41 PM
0003-saml2-store-current-value-of-OPENED_SESSION_COOKIE-i.patch (1.62 KB) 0003-saml2-store-current-value-of-OPENED_SESSION_COOKIE-i.patch Benjamin Dauvergne, 15 December 2022 05:41 PM
0001-misc-replace-deprecated-distutils-features-by-setupt.patch (2.06 KB) 0001-misc-replace-deprecated-distutils-features-by-setupt.patch Benjamin Dauvergne, 15 December 2022 05:41 PM
0002-misc-retry-passive-sso-when-session-expire-and-logou.patch (7.06 KB) 0002-misc-retry-passive-sso-when-session-expire-and-logou.patch Benjamin Dauvergne, 15 December 2022 05:41 PM
0005-test-modify-test-to-test-on-a-real-form-67090.patch (3.51 KB) 0005-test-modify-test-to-test-on-a-real-form-67090.patch Benjamin Dauvergne, 15 December 2022 05:55 PM
0004-tests-use-cookie-name-in-get_session-app-67090.patch (1.29 KB) 0004-tests-use-cookie-name-in-get_session-app-67090.patch Benjamin Dauvergne, 15 December 2022 05:55 PM
0003-saml2-store-current-value-of-OPENED_SESSION_COOKIE-i.patch (1.62 KB) 0003-saml2-store-current-value-of-OPENED_SESSION_COOKIE-i.patch Benjamin Dauvergne, 15 December 2022 05:55 PM
0001-misc-replace-deprecated-distutils-features-by-setupt.patch (2.06 KB) 0001-misc-replace-deprecated-distutils-features-by-setupt.patch Benjamin Dauvergne, 15 December 2022 05:55 PM
0002-misc-retry-passive-sso-when-session-expire-and-logou.patch (7.06 KB) 0002-misc-retry-passive-sso-when-session-expire-and-logou.patch Benjamin Dauvergne, 15 December 2022 05:55 PM
0002-tests-use-cookie-name-in-get_session-app-67090.patch (1.1 KB) 0002-tests-use-cookie-name-in-get_session-app-67090.patch Benjamin Dauvergne, 20 December 2022 11:17 AM
0001-misc-improve-passive-sso-on-state-change-67090.patch (9.27 KB) 0001-misc-improve-passive-sso-on-state-change-67090.patch Benjamin Dauvergne, 20 December 2022 11:17 AM
0003-test-modify-test-to-test-on-a-real-form-67090.patch (3.51 KB) 0003-test-modify-test-to-test-on-a-real-form-67090.patch Benjamin Dauvergne, 20 December 2022 11:17 AM
0004-test-modify-test-to-test-on-a-real-form-67090.patch (3.51 KB) 0004-test-modify-test-to-test-on-a-real-form-67090.patch Benjamin Dauvergne, 23 December 2022 11:01 AM
0003-tests-use-cookie-name-in-get_session-app-67090.patch (1.1 KB) 0003-tests-use-cookie-name-in-get_session-app-67090.patch Benjamin Dauvergne, 23 December 2022 11:01 AM
0002-misc-improve-passive-sso-on-state-change-67090.patch (9.69 KB) 0002-misc-improve-passive-sso-on-state-change-67090.patch Benjamin Dauvergne, 23 December 2022 11:01 AM
0001-misc-let-django-generate-set-cookies-headers-72613.patch (8.05 KB) 0001-misc-let-django-generate-set-cookies-headers-72613.patch Benjamin Dauvergne, 23 December 2022 11:01 AM

Related issues

Related to django-mellon - Development #66747: middleware: utiliser la valeur du cookie MELLON_OPENED_SESSION comme référence à la session globale en coursFermé28 June 2022

Actions
Related to django-mellon - Development #67084: middleware: retiré MELLON_PASSIVE_TRIED si on est connectéFermé06 July 2022

Actions
Related to w.c.s. - Development #72613: Problème avec plusieurs Set-cookie dans le code conversion Quixote et DjangoSolution déployée19 December 2022

Actions

Associated revisions

Revision 6a4cea8b (diff)
Added by Benjamin Dauvergne about 1 month ago

misc: improve passive sso on state change (#67090)

- automatic_sso is renamed try_passive_sso to be clearer on the goal of
the method,
- test for possible passive sso is now done before rendering the current
page,
- on a succesfull SSO if <idp_session_cookie_name> cookie is present,
its value is saved in the Quixote session,
- behaviour of try_passive_sso is changed:
- if user is logged in or <idp_session_cookie_name> cookie value
differs from the value in '*-passive-auth-tried' cookie, the
'*-passive-auth-tried' cookie is expired,
- if user is logged and <idp_session_cookie_name> cookie cookie value
differs from the one saved in the Quixote session, user is logged
out, if not treatment stop here.
- if the <idp_session_cookie_name> cookie cookie is not valued or if
its value is equal to '*-passive-auth-tried' cookie, treatment stops
here,
- if the <idp_session_cookie_name> cookie is valued and its value
differs from the '*-passive-auth-tried' cookie cookie value, then
'*-passive-auth-tried' cookie is set to the value of
<idp_session_cookie_name> cookie and a passive SSO is tried.

Revision bc4d9e17 (diff)
Added by Benjamin Dauvergne about 1 month ago

tests: use cookie name in get_session(app) (#67090)

Revision 8880ab0d (diff)
Added by Benjamin Dauvergne about 1 month ago

test: modify test to test on a real form (#67090)

History

#2

Updated by Frédéric Péters 7 months ago

Les tickets cités manquent de contexte, offrent juste une explication technique de ce que font les patchs, c'est pour répondre à quel problème ?

#3

Updated by Benjamin Dauvergne 7 months ago

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

Les tickets cités manquent de contexte, offrent juste une explication technique de ce que font les patchs, c'est pour répondre à quel problème ?

Deux problèmes différents :
1. se trouver sur w.c.s. non connecté alors qu'une session existe sur authentic (ça arrive si la session coté w.c.s. expire avant celle d'authentic, ce qui est normalement garanti même), dans ce cas le cookie PASSIVE_TRIED_COOKIE était toujours présent empêchant de relancer un SSO passif (ce qui rallonge la session coté IdP, etc...),
2. se trouver connecté sur w.c.s avec un utilisateur différent de celui avec lequel on est connecté sur authentic (le SLO a foiré, le cookie sur authentic a expiré avant celui de w.c.s. pour une raison x ou y) ou alors qu'on est plus connecté sur authentic.

Ça corrige les deux problèmes, le code est quasi identique à celui du middleware qui fait ça dans django-melon.

Ce sont des choses remontés par des tickets GLC lors des tests de GLCPRO (ça concernait surtout combo mais le problème est présent sur w.c.s. aussi).

#4

Updated by Benjamin Dauvergne 3 months ago

  • Related to Development #66747: middleware: utiliser la valeur du cookie MELLON_OPENED_SESSION comme référence à la session globale en cours added
#6

Updated by Benjamin Dauvergne 3 months ago

  • Related to Development #67084: middleware: retiré MELLON_PASSIVE_TRIED si on est connecté added
#7

Updated by Benjamin Dauvergne 3 months ago

Pour référence le ticket #65431 parle d'une session ouverte sur le portail alors qu'il n'y en a plus sur authentic, mais le même comportement était observé sur w.c.s.

#8

Updated by Benjamin Dauvergne about 2 months ago

Rebasé.

#10

Updated by Benjamin Dauvergne about 1 month ago

  • Tracker changed from Development to Bug
  • Description updated (diff)
#12

Updated by Frédéric Péters about 1 month ago

Il y a cinq patchs attachés mais dans la branche il y a 4 commits ? (ok... c'est 0001 qui s'est perdu ici).

Le sujet du commit 0002 est bien trop long.

Dans ce qui semble un changement d'indentation se glisse au moins une modification (secure=request.scheme == 'https'), ça aiderait la relecture de pointer ce qu'il pourrait y avoir d'autre.

Dans 0003 idéalement les nouveaux attributs sont intégrés dans has_info().

Dans 0004 au-delà de la description donnée dans le sujet il y a un ignore_errors=True qui s'est glissé.

#13

Updated by Benjamin Dauvergne about 1 month ago

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

Le sujet du commit 0002 est bien trop long.

Ok, je revois ça.

Dans ce qui semble un changement d'indentation se glisse au moins une modification (secure=request.scheme 'https'), ça aiderait la relecture de pointer ce qu'il pourrait y avoir d'autre.

J'ai mergé 0003 dans 0002, les deux ne fonctionnent pas sans l'autre de toute façon et j'ai réduit le titre du commit et mis une description plus exhaustive des changements de comportement.

Le changement secure.request.scheme 'https' c'est pour que ça fonctionne dans les tests: comme webtest simule des requêtes http cookiejar refusait de servir les cookies.

Dans 0003 idéalement les nouveaux attributs sont intégrés dans has_info().

Ok je l'ajoute pour la forme, de fait sur un login il y a tellement de choses qui changent dans la session qu'elle est toujours sauvée (ce mécanisme has_info n'est pas terrible, un jour il faudra voir à faire un truc plus proche de ce que fait Django qui essaye de détecter les modifications).

Dans 0004 au-delà de la description donnée dans le sujet il y a un ignore_errors=True qui s'est glissé.

Ce changement est retiré, ça n'est pas/plus nécessaire, j'ai bataillé avec les cookies de session, c'est devenu #72613 (la branche est rebasée sur #72613).

#15

Updated by Frédéric Péters about 1 month ago

+            OPENED_SESSION_COOKIE = get_publisher().get_site_option('idp_session_cookie_name')
+            if OPENED_SESSION_COOKIE:
+                if OPENED_SESSION_COOKIE in get_request().cookies:
+                    session.opened_session_value = get_request().cookies[OPENED_SESSION_COOKIE]

Tant qu'à avoir tout ce code qui change (ici et ailleurs), ça pourrait être l'occasion pour écrire en minuscules, comme des variables classiques ? (ça aurait en bonus de sans doute produire un diff plus lisible, qui n'intercalerait pas quelques lignes d'ancien code au milieu du nouveau).

Sur le fond j'ai un peu peur de la partie

  - if user is logged and IDP_OPENED_SESSION cookie value differs from
    the one saved in the Quixote session, user is logged out, if not
    treatment stop here.

mais elle existe dans django-mellon et ça semble passer là, donc ok.

#17

Updated by Frédéric Péters about 1 month ago

  • Related to Development #72613: Problème avec plusieurs Set-cookie dans le code conversion Quixote et Django added
#18

Updated by Frédéric Péters about 1 month ago

  • Status changed from Solution proposée to Résolu (à déployer)
commit 8880ab0d6a06cc622748a1beb7bc667ed395418d
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Thu Dec 15 17:32:25 2022 +0100

    test: modify test to test on a real form (#67090)

commit bc4d9e17f10065d6774fd0aeea8492c42ad95c44
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Wed Jul 6 19:02:23 2022 +0200

    tests: use cookie name in get_session(app) (#67090)

commit 6a4cea8b30c7a3d47f4bb0030c182005c07d8554
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Wed Jul 6 19:06:03 2022 +0200

    misc: improve passive sso on state change (#67090)

    - automatic_sso is renamed try_passive_sso to be clearer on the goal of
      the method,
    - test for possible passive sso is now done before rendering the current
      page,
    - on a succesfull SSO if <idp_session_cookie_name> cookie is present,
      its value is saved in the Quixote session,
    - behaviour of try_passive_sso is changed:
      - if user is logged in or <idp_session_cookie_name> cookie value
        differs from the value in '*-passive-auth-tried' cookie, the
        '*-passive-auth-tried' cookie is expired,
      - if user is logged and <idp_session_cookie_name> cookie cookie value
        differs from the one saved in the Quixote session, user is logged
        out, if not treatment stop here.
      - if the <idp_session_cookie_name> cookie cookie is not valued or if
        its value is equal to '*-passive-auth-tried' cookie, treatment stops here,
      - if the <idp_session_cookie_name> cookie is valued and its value
        differs from the '*-passive-auth-tried' cookie cookie value, then
        '*-passive-auth-tried' cookie is set to the value of
        <idp_session_cookie_name> cookie and a passive SSO is tried.
#19

Updated by Transition automatique about 1 month ago

  • Status changed from Résolu (à déployer) to Solution déployée
#20

Updated by Gitea (Bot) Gitea 22 days ago

  • Assignee changed from Benjamin Dauvergne to Frédéric Péters
#21

Updated by Gitea (Bot) Gitea 22 days ago

Frédéric Péters (fpeters) a lié une pull request sur Gitea concernant cette demande :

#22

Updated by Gitea (Bot) Gitea 22 days ago

Frédéric Péters (fpeters) a mergé une pull request sur Gitea concernant cette demande :

Also available in: Atom PDF