From 0e3e73989d55699c0ddfa45a626ad0b331fed086 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Wed, 8 Jul 2020 16:18:02 +0200 Subject: [PATCH] sources/saml: Add NameID Policy field, sent with AuthnRequest --- passbook/sources/saml/api.py | 1 + passbook/sources/saml/forms.py | 1 + .../0005_samlsource_name_id_policy.py | 40 +++++++++++++++++++ passbook/sources/saml/models.py | 24 +++++++++++ passbook/sources/saml/processors/base.py | 7 ++++ .../templates/saml/sp/xml/authn_request.xml | 19 ++++----- passbook/sources/saml/views.py | 1 + swagger.yaml | 11 +++++ 8 files changed, 95 insertions(+), 9 deletions(-) create mode 100644 passbook/sources/saml/migrations/0005_samlsource_name_id_policy.py diff --git a/passbook/sources/saml/api.py b/passbook/sources/saml/api.py index b594799e8..3e456f52e 100644 --- a/passbook/sources/saml/api.py +++ b/passbook/sources/saml/api.py @@ -15,6 +15,7 @@ class SAMLSourceSerializer(ModelSerializer): fields = SOURCE_FORM_FIELDS + [ "issuer", "sso_url", + "name_id_policy", "binding_type", "slo_url", "temporary_user_delete_after", diff --git a/passbook/sources/saml/forms.py b/passbook/sources/saml/forms.py index 413b8e9bc..6fe3c0c83 100644 --- a/passbook/sources/saml/forms.py +++ b/passbook/sources/saml/forms.py @@ -23,6 +23,7 @@ class SAMLSourceForm(forms.ModelForm): fields = SOURCE_FORM_FIELDS + [ "issuer", "sso_url", + "name_id_policy", "binding_type", "slo_url", "temporary_user_delete_after", diff --git a/passbook/sources/saml/migrations/0005_samlsource_name_id_policy.py b/passbook/sources/saml/migrations/0005_samlsource_name_id_policy.py new file mode 100644 index 000000000..5b5e6f649 --- /dev/null +++ b/passbook/sources/saml/migrations/0005_samlsource_name_id_policy.py @@ -0,0 +1,40 @@ +# Generated by Django 3.0.8 on 2020-07-08 13:26 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("passbook_sources_saml", "0004_auto_20200708_1207"), + ] + + operations = [ + migrations.AddField( + model_name="samlsource", + name="name_id_policy", + field=models.TextField( + choices=[ + ("urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress", "Email"), + ( + "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent", + "Persistent", + ), + ( + "urn:oasis:names:tc:SAML:2.0:nameid-format:X509SubjectName", + "X509", + ), + ( + "urn:oasis:names:tc:SAML:2.0:nameid-format:WindowsDomainQualifiedName", + "Windows", + ), + ( + "urn:oasis:names:tc:SAML:2.0:nameid-format:transient", + "Transient", + ), + ], + default="urn:oasis:names:tc:SAML:2.0:nameid-format:transient", + help_text="NameID Policy sent to the IdP. Can be unset, in which case no Policy is sent.", + ), + ), + ] diff --git a/passbook/sources/saml/models.py b/passbook/sources/saml/models.py index b81da562e..1f308b70f 100644 --- a/passbook/sources/saml/models.py +++ b/passbook/sources/saml/models.py @@ -7,6 +7,13 @@ from passbook.core.models import Source from passbook.core.types import UILoginButton from passbook.crypto.models import CertificateKeyPair from passbook.providers.saml.utils.time import timedelta_string_validator +from passbook.sources.saml.processors.constants import ( + SAML_NAME_ID_FORMAT_EMAIL, + SAML_NAME_ID_FORMAT_PRESISTENT, + SAML_NAME_ID_FORMAT_TRANSIENT, + SAML_NAME_ID_FORMAT_WINDOWS, + SAML_NAME_ID_FORMAT_X509, +) class SAMLBindingTypes(models.TextChoices): @@ -17,6 +24,16 @@ class SAMLBindingTypes(models.TextChoices): POST_AUTO = "POST_AUTO", _("POST Binding with auto-confirmation") +class SAMLNameIDPolicy(models.TextChoices): + """SAML NameID Policies""" + + EMAIL = SAML_NAME_ID_FORMAT_EMAIL + PERSISTENT = SAML_NAME_ID_FORMAT_PRESISTENT + X509 = SAML_NAME_ID_FORMAT_X509 + WINDOWS = SAML_NAME_ID_FORMAT_WINDOWS + TRANSIENT = SAML_NAME_ID_FORMAT_TRANSIENT + + class SAMLSource(Source): """Authenticate using an external SAML Identity Provider.""" @@ -31,6 +48,13 @@ class SAMLSource(Source): verbose_name=_("SSO URL"), help_text=_("URL that the initial Login request is sent to."), ) + name_id_policy = models.TextField( + choices=SAMLNameIDPolicy.choices, + default=SAMLNameIDPolicy.TRANSIENT, + help_text=_( + "NameID Policy sent to the IdP. Can be unset, in which case no Policy is sent." + ), + ) binding_type = models.CharField( max_length=100, choices=SAMLBindingTypes.choices, diff --git a/passbook/sources/saml/processors/base.py b/passbook/sources/saml/processors/base.py index 036c034f5..c1763bd81 100644 --- a/passbook/sources/saml/processors/base.py +++ b/passbook/sources/saml/processors/base.py @@ -127,6 +127,13 @@ class Processor: def prepare_flow(self, request: HttpRequest) -> HttpResponse: """Prepare flow plan depending on whether or not the user exists""" name_id = self._get_name_id() + # Sanity check, show a warning if NameIDPolicy doesn't match what we go + if self._source.name_id_policy != name_id.attrib["Format"]: + LOGGER.warning( + "NameID from IdP doesn't match our policy", + expected=self._source.name_id_policy, + got=name_id.attrib["Format"], + ) # transient NameIDs are handeled seperately as they don't have to go through flows. if name_id.attrib["Format"] == SAML_NAME_ID_FORMAT_TRANSIENT: return self._handle_name_id_transient(request) diff --git a/passbook/sources/saml/templates/saml/sp/xml/authn_request.xml b/passbook/sources/saml/templates/saml/sp/xml/authn_request.xml index 724a77753..64b7c454b 100644 --- a/passbook/sources/saml/templates/saml/sp/xml/authn_request.xml +++ b/passbook/sources/saml/templates/saml/sp/xml/authn_request.xml @@ -1,11 +1,12 @@ - - {{ ISSUER }} - {{ AUTHN_REQUEST_SIGNATURE }} + + {{ ISSUER }} + {{ AUTHN_REQUEST_SIGNATURE }} + diff --git a/passbook/sources/saml/views.py b/passbook/sources/saml/views.py index f734ea5c5..105e99a7e 100644 --- a/passbook/sources/saml/views.py +++ b/passbook/sources/saml/views.py @@ -41,6 +41,7 @@ class InitiateView(View): "AUTHN_REQUEST_ID": get_random_id(), "ISSUE_INSTANT": get_time_string(), "ISSUER": get_issuer(request, source), + "NAME_ID_POLICY": source.name_id_policy, } authn_req = get_authnrequest_xml(parameters, signed=False) # If the source is configured for Redirect bindings, we can just redirect there diff --git a/swagger.yaml b/swagger.yaml index 18c119571..3cfe4a277 100755 --- a/swagger.yaml +++ b/swagger.yaml @@ -6505,6 +6505,17 @@ definitions: format: uri maxLength: 200 minLength: 1 + name_id_policy: + title: Name id policy + description: NameID Policy sent to the IdP. Can be unset, in which case no + Policy is sent. + type: string + enum: + - urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress + - urn:oasis:names:tc:SAML:2.0:nameid-format:persistent + - urn:oasis:names:tc:SAML:2.0:nameid-format:X509SubjectName + - urn:oasis:names:tc:SAML:2.0:nameid-format:WindowsDomainQualifiedName + - urn:oasis:names:tc:SAML:2.0:nameid-format:transient binding_type: title: Binding type type: string