From 439a1ed76d15a77911958bde14cea3b2bd6c017b Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Mon, 3 Oct 2022 17:23:51 +0200 Subject: [PATCH 6/8] misc: make logout work with transient NameID (#69740) Implementation of transient NameID is special, the transient NameID is ignored and an attribut value is used as the federation key. But in order to producre a proper NameID for the logout request we need the transient NameID value. To work around this problem we add a transient_name_id attribute to the SessionIndex model representing the current SSO session, and we modify the session dump template to use this value instead of UserSAMLIdentifier.name_id if transient_name_id is not None. --- mellon/adapters.py | 6 +++- .../0007_sessionindex_transient_name_id.py | 18 ++++++++++ mellon/models.py | 1 + mellon/templates/mellon/session_dump.xml | 2 +- mellon/views.py | 4 +++ tests/test_sso_slo.py | 35 +++++++++++++++++-- 6 files changed, 62 insertions(+), 4 deletions(-) create mode 100644 mellon/migrations/0007_sessionindex_transient_name_id.py diff --git a/mellon/adapters.py b/mellon/adapters.py index 1778926..0f5b98e 100644 --- a/mellon/adapters.py +++ b/mellon/adapters.py @@ -316,6 +316,7 @@ class DefaultAdapter: transient_federation_attribute, ) return None + saml_attributes['transient_name_id_content'] = name_id else: if self.request: messages.warning( @@ -459,8 +460,11 @@ class DefaultAdapter: return None def _link_user(self, idp, saml_attributes, user): + name_id_content = saml_attributes['name_id_content'] + if saml_attributes['name_id_format'] == lasso.SAML2_NAME_IDENTIFIER_FORMAT_TRANSIENT: + name_id_content = saml_attributes['transient_name_id_content'] saml_id, created = models.UserSAMLIdentifier.objects.get_or_create( - name_id=saml_attributes['name_id_content'], + name_id=name_id_content, issuer=models_utils.get_issuer(saml_attributes['issuer']), defaults={ 'user': user, diff --git a/mellon/migrations/0007_sessionindex_transient_name_id.py b/mellon/migrations/0007_sessionindex_transient_name_id.py new file mode 100644 index 0000000..607cbc5 --- /dev/null +++ b/mellon/migrations/0007_sessionindex_transient_name_id.py @@ -0,0 +1,18 @@ +# Generated by Django 2.2.26 on 2022-10-03 15:23 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('mellon', '0006_nameid_attributes'), + ] + + operations = [ + migrations.AddField( + model_name='sessionindex', + name='transient_name_id', + field=models.TextField(null=True, verbose_name='Transient NameID'), + ), + ] diff --git a/mellon/models.py b/mellon/models.py index 11b1c76..13bf36d 100644 --- a/mellon/models.py +++ b/mellon/models.py @@ -46,6 +46,7 @@ class UserSAMLIdentifier(models.Model): class SessionIndex(models.Model): session_index = models.TextField(_('SAML SessionIndex')) session_key = models.CharField(_('Django session key'), max_length=40) + transient_name_id = models.TextField(verbose_name=_('Transient NameID'), null=True) saml_identifier = models.ForeignKey( verbose_name=_('SAML identifier'), to=UserSAMLIdentifier, on_delete=models.CASCADE ) diff --git a/mellon/templates/mellon/session_dump.xml b/mellon/templates/mellon/session_dump.xml index a7305e9..450a676 100644 --- a/mellon/templates/mellon/session_dump.xml +++ b/mellon/templates/mellon/session_dump.xml @@ -1,5 +1,5 @@ {% for session_index in session_indexes %}{% with nameid=session_index.saml_identifier %} -{{ nameid.name_id }} +{% firstof session_index.transient_name_id nameid.name_id %} {% endwith %}{% endfor %} diff --git a/mellon/views.py b/mellon/views.py index 550af8c..13d7553 100644 --- a/mellon/views.py +++ b/mellon/views.py @@ -355,6 +355,10 @@ class LoginView(ProfileMixin, LogMixin, View): saml_identifier=user.saml_identifier, session_key=self.request.session.session_key, session_index=session_index, + # keep transient nameid to be able to produce logout requests + transient_name_id=attributes['name_id_content'] + if attributes['name_id_format'] == lasso.SAML2_NAME_IDENTIFIER_FORMAT_TRANSIENT + else None, ) self.log.info('user %s (NameID is %r) logged in using SAML', user, attributes['name_id_content']) self.request.session['mellon_session'] = utils.flatten_datetime(attributes) diff --git a/tests/test_sso_slo.py b/tests/test_sso_slo.py index e8d947c..08b202e 100644 --- a/tests/test_sso_slo.py +++ b/tests/test_sso_slo.py @@ -16,6 +16,7 @@ import base64 import datetime +import logging import re import urllib.parse as urlparse import xml.etree.ElementTree as ET @@ -33,6 +34,7 @@ from httmock import HTTMock, all_requests from httmock import response as mock_response from pytest import fixture +from mellon import models from mellon.utils import create_metadata from mellon.views import lasso_decode @@ -82,7 +84,7 @@ class MockIdp: session_dump = None identity_dump = None - def __init__(self, idp_metadata, private_key, sp_metadata): + def __init__(self, idp_metadata, private_key, sp_metadata, name_id=None): self.server = server = lasso.Server.newFromBuffers(idp_metadata, private_key) self.server.signatureMethod = lasso.SIGNATURE_METHOD_RSA_SHA256 server.addProviderFromBuffer(lasso.PROVIDER_ROLE_SP, sp_metadata) @@ -90,7 +92,7 @@ class MockIdp: def reset_session_dump(self): self.session_dump = None - def process_authn_request_redirect(self, url, auth_result=True, consent=True, msg=None): + def process_authn_request_redirect(self, url, auth_result=True, consent=True, msg=None, name_id=None): login = self.login = lasso.Login(self.server) if self.identity_dump: login.setIdentityFromDump(self.identity_dump) @@ -121,6 +123,8 @@ class MockIdp: datetime.datetime.now().isoformat(), datetime.datetime.now().isoformat(), ) + for key in name_id or {}: + setattr(login.assertion.subject.nameID, key, name_id[key]) def add_attribute(name, *values, **kwargs): fmt = kwargs.get('fmt', lasso.SAML2_ATTRIBUTE_NAME_FORMAT_BASIC) @@ -833,3 +837,30 @@ def test_force_authn(db, app, idp, caplog, sp_settings): response = app.post(reverse('mellon_login'), params={'SAMLResponse': body, 'RelayState': relay_state}) assert app.session['mellon_session']['force_authn'] + + +def test_sso_slo_transient_name_identifier(db, app, idp, caplog, sp_settings): + caplog.set_level(logging.WARNING) + sp_settings.MELLON_TRANSIENT_FEDERATION_ATTRIBUTE = 'email' + response = app.get('/login/') + url, body, relay_state = idp.process_authn_request_redirect( + response['Location'], + name_id={ + 'format': lasso.SAML2_NAME_IDENTIFIER_FORMAT_TRANSIENT, + 'content': '1234', + }, + ) + response = app.post('/login/', params={'SAMLResponse': body, 'RelayState': relay_state}) + + usi = models.UserSAMLIdentifier.objects.get() + assert usi.name_id == 'john.doe@gmail.com' + session_index = models.SessionIndex.objects.get(saml_identifier=usi) + assert session_index.transient_name_id == '1234' + + response = app.get('/logout/') + assert urlparse.urlparse(response['Location']).path == '/singleLogout' + url = idp.process_logout_request_redirect(response.location) + caplog.clear() + response = app.get(url) + assert len(caplog.records) == 0, 'logout failed' + assert response.location == '/' -- 2.37.2