Only add NameIdPolicy if NameIdFormat is set (#2)

mattermost · Oct 7, 2020 · 4e9a72d · 4e9a72d
1 parent 8380e0f
commit 4e9a72d
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 3 deletions.
diff --git a/build_request.go b/build_request.go
@@ -41,9 +41,11 @@ func (sp *SAMLServiceProvider) buildAuthnRequest(includeSig bool) (*etree.Docume
 		authnRequest.CreateElement("saml:Issuer").SetText(sp.IdentityProviderIssuer)
 	}
 
-	nameIdPolicy := authnRequest.CreateElement("samlp:NameIDPolicy")
-	nameIdPolicy.CreateAttr("AllowCreate", "true")
-	nameIdPolicy.CreateAttr("Format", sp.NameIdFormat)
+	if sp.NameIdFormat != "" {
+		nameIdPolicy := authnRequest.CreateElement("samlp:NameIDPolicy")
+		nameIdPolicy.CreateAttr("AllowCreate", "true")
+		nameIdPolicy.CreateAttr("Format", sp.NameIdFormat)
+	}
 
 	if sp.RequestedAuthnContext != nil {
 		requestedAuthnContext := authnRequest.CreateElement("samlp:RequestedAuthnContext")

diff --git a/build_request_test.go b/build_request_test.go
@@ -175,3 +175,48 @@ func TestScopingIDProviderOmitted(t *testing.T) {
 		require.Nil(t, el)
 	}
 }
+
+func TestScopingNameIDPolicyIncluded(t *testing.T) {
+	spURL := "https://sp.test"
+	sp := SAMLServiceProvider{
+		AssertionConsumerServiceURL: spURL,
+		AudienceURI:                 spURL,
+		IdentityProviderIssuer:      spURL,
+		IdentityProviderSSOURL:      "https://idp.test/saml/sso",
+		SignAuthnRequests:           false,
+		NameIdFormat:                NameIdFormatPersistent,
+	}
+
+	request, err := sp.BuildAuthRequest()
+	require.NoError(t, err)
+
+	doc := etree.NewDocument()
+	err = doc.ReadFromString(request)
+	require.NoError(t, err)
+
+	idpEntry := doc.FindElement("./AuthnRequest/NameIDPolicy")
+
+	require.Equal(t, idpEntry.SelectAttrValue("Format", ""), NameIdFormatPersistent)
+}
+
+func TestScopingNameIDPolicyOmitted(t *testing.T) {
+	spURL := "https://sp.test"
+
+	sp := SAMLServiceProvider{
+		AssertionConsumerServiceURL: spURL,
+		AudienceURI:                 spURL,
+		IdentityProviderIssuer:      spURL,
+		IdentityProviderSSOURL:      "https://idp.test/saml/sso",
+		SignAuthnRequests:           false,
+	}
+
+	request, err := sp.BuildAuthRequest()
+	require.NoError(t, err)
+
+	doc := etree.NewDocument()
+	err = doc.ReadFromString(request)
+	require.NoError(t, err)
+
+	el := doc.FindElement("./AuthnRequest/NameIDPolicy")
+	require.Nil(t, el)
+}