Project

General

Profile

Bug #84017

Une configuration OIDC avec des redirect_uris ne passe plus, même sans sector_identifier

Added by Frédéric Péters 3 months ago. Updated about 2 months ago.

Status:
Solution déployée
Priority:
Normal
Category:
-
Target version:
-
Start date:
28 November 2023
Due date:
% Done:

0%

Estimated time:
Patch proposed:
No
Planning:
No

Description

La validation "all redirect_uri do not have the same hostname" s'applique désormais (depuis #83365) à un moment où ça n'était pas le cas avant, parce que le get_session_id() est appelé depuis un nouvel endroit :

ValueError: all redirect_uri do not have the same hostname
  File "django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "django/core/handlers/base.py", line 181, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "authentic2/decorators.py", line 40, in f
    return func(request, *args, **kwargs)
  File "django/views/decorators/csrf.py", line 54, in wrapped_view
    return view_func(*args, **kwargs)
  File "authentic2_idp_oidc/views.py", line 842, in token
    response = tokens_from_authz_code(request)
  File "authentic2_idp_oidc/views.py", line 815, in tokens_from_authz_code
    'sid': utils.get_session_id(request.session, client),
  File "authentic2_idp_oidc/utils.py", line 284, in get_session_id
    sector_identifier = force_bytes(client.get_sector_identifier())
  File "authentic2_idp_oidc/models.py", line 233, in get_sector_identifier
    raise ValueError('all redirect_uri do not have the same hostname')

https://sentry.entrouvert.org/entrouvert/publik/issues/117075/


Related issues

Related to Authentic 2 - Bug #84092: idp_oidc : l’édition BO d’un client OIDC devrait échouer sur des URIs de redirection portant des noms de domaine différents tandis qu’aucune URI d’identifiant de secteur n’est définieFermé30 November 2023

Actions

Associated revisions

Revision 1d92a060 (diff)
Added by Benjamin Dauvergne about 2 months ago

idp_oidc: build the sid using the client_id instead of the sector identifier (#84017)

Revision 28afd8c4 (diff)
Added by Benjamin Dauvergne about 2 months ago

tox.ini: use pytest-cov test context (#84017)

History

#2

Updated by Benjamin Dauvergne 3 months ago

Je peux changer la façon de calculer le sid pour pallier à cette régression, dans l'attente il faut soit supprimer les URLs avec domaine différent (ce qui a été fait sur clermont, bien) soit choisir une URL tronquée à la racine au hasard (la première) et la définir comme sector_identifier.

#3

Updated by Benjamin Dauvergne 3 months ago

  • Assignee set to Benjamin Dauvergne
#4

Updated by Robot Gitea 3 months ago

  • Status changed from Nouveau to Solution proposée

Benjamin Dauvergne (bdauvergne) a ouvert une pull request sur Gitea concernant cette demande :

#5

Updated by Benjamin Dauvergne 3 months ago

J'ai choisi d'utiliser le client_id plutôt que le sector_identifier comme élément différenciant les sid entre RP OIDC, sachant qu'à ma connaissance personne n'utilise le sid à part une demande recette pas encore en prod, ça ne devrait pas provoquer trop de souci au niveau des déconnexions, en tout cas rien qui persiste.

PS: pour préciser on envoie le sid dans les requêtes frontchannel de déconnexion vers les RPs si le RP valide le sid par rapport à ce qu'il a reçu ça pourrait bloquer une requête de déconnexion.

PS2: ça n'aura pas d'effet sur les sessions en cours, l'URL de déconnexion étant prégénérée au moment du SSO et stockée en session.

#7

Updated by Abdessamad Assila 3 months ago

Merci Fréd. Est-ce que c'est possible de le déployer encore ce matin? merci

#8

Updated by Paul Marillonnet 3 months ago

  • Related to Bug #84092: idp_oidc : l’édition BO d’un client OIDC devrait échouer sur des URIs de redirection portant des noms de domaine différents tandis qu’aucune URI d’identifiant de secteur n’est définie added
#9

Updated by Robot Gitea 2 months ago

  • Status changed from Solution proposée to En cours

Thomas NOËL (tnoel) a relu et demandé des modifications sur une pull request sur Gitea concernant cette demande :

#10

Updated by Benjamin Dauvergne 2 months ago

  • Status changed from En cours to Solution proposée
#11

Updated by Robot Gitea 2 months ago

  • Status changed from Solution proposée to Solution validée

Thomas NOËL (tnoel) a approuvé une pull request sur Gitea concernant cette demande :

#12

Updated by Robot Gitea about 2 months ago

  • Status changed from Solution validée to Résolu (à déployer)

Benjamin Dauvergne (bdauvergne) a mergé une pull request sur Gitea concernant cette demande :

#15

Updated by Transition automatique about 2 months ago

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

Also available in: Atom PDF