From a43e3734b20a4a02901679cca647d2f920d9ccb9 Mon Sep 17 00:00:00 2001 From: Sacha Moufarrege Date: Wed, 17 Sep 2025 13:31:24 +0200 Subject: [PATCH 1/5] SAML-26: Return multiple AttributeValues if array of values is found --- .../saml/response/SamlResponseBuilder.cfc | 8 +++- tests/unit/ResponseBuilderTest.cfc | 42 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/services/saml/response/SamlResponseBuilder.cfc b/services/saml/response/SamlResponseBuilder.cfc index 079a915..70053ca 100644 --- a/services/saml/response/SamlResponseBuilder.cfc +++ b/services/saml/response/SamlResponseBuilder.cfc @@ -235,8 +235,14 @@ component { } else { xml &= ''; } + if ( IsArray( arguments.attributes[ key ] ) ) { + for ( var val in arguments.attributes[ key ] ) { + xml &= '#XmlFormat( val )#'; + } + } else { + xml &= '#XmlFormat( arguments.attributes[ key ] )#'; + } - xml &= '#XmlFormat( arguments.attributes[ key ] )#'; xml &= ''; } diff --git a/tests/unit/ResponseBuilderTest.cfc b/tests/unit/ResponseBuilderTest.cfc index ac016da..79030a2 100644 --- a/tests/unit/ResponseBuilderTest.cfc +++ b/tests/unit/ResponseBuilderTest.cfc @@ -56,6 +56,48 @@ component extends="testbox.system.BaseSpec" { fail( "SAML did not validate" ); } } ); + + it( "should handle attributes with array values", function(){ + var builder = _getBuilder(); + var response = builder.buildAuthenticationAssertion( + issuer = "http://www.thewebsite.com/" + , nameIdFormat = "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent" + , nameIdValue = "test@test.com" + , inResponseTo = "aaf23196-1773-2113-474a-fe114412ab72" + , recipientUrl = "https://sp.example.com/SAML2/SSO/POST" + , audience = "https://sp.example.com/SAML2" + , sessionTimeout = 30 + , sessionIndex = "C894146D-598F-4D9B-8733ACF80280C4B7" + , attributes = { + email = "test@test.com", + displayName="Test user", + firstName="Test", + lastName="user", + roles = ["expert", "admin"] + } + , privateKey = testPk + , publicCertificate = testCert + ); + + expect( IsXml( response ) ).toBeTrue(); + expect( response ).toInclude( "" ); + expect( response ).toInclude( "]*>expert", "all" ).len(); + expect( attributeValueCount ).toBe( 1 ); + attributeValueCount = ( response ).reFindNoCase( "]*>admin", "all" ).len(); + expect( attributeValueCount ).toBe( 1 ); + + var openSamlObjectRepresentingResponse = openSamlUtils.xmlToOpenSamlObject( response ); + try { + openSamlObjectRepresentingResponse.validate( true ); + } catch ( any e ) { + fail( "SAML did not validate" ); + } + } ); } ); describe( "buildErrorResponse()", function(){ From 0163ab74693ed28d12b54140b88ebfab01290ede Mon Sep 17 00:00:00 2001 From: Sacha Moufarrege Date: Wed, 17 Sep 2025 13:42:07 +0200 Subject: [PATCH 2/5] SAML-26: Fix issue with test --- tests/unit/ResponseBuilderTest.cfc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/ResponseBuilderTest.cfc b/tests/unit/ResponseBuilderTest.cfc index 79030a2..e793ff3 100644 --- a/tests/unit/ResponseBuilderTest.cfc +++ b/tests/unit/ResponseBuilderTest.cfc @@ -86,10 +86,10 @@ component extends="testbox.system.BaseSpec" { expect( response ).toInclude( "admin" ); // Count the number of AttributeValue elements for roles - var attributeValueCount = ( response ).reFindNoCase( "]*>expert", "all" ).len(); - expect( attributeValueCount ).toBe( 1 ); - attributeValueCount = ( response ).reFindNoCase( "]*>admin", "all" ).len(); - expect( attributeValueCount ).toBe( 1 ); + var expertMatches = ( response ).reFindNoCase( "]*>expert", "all" ); + expect( ArrayLen( expertMatches ) ).toBe( 1 ); + var adminMatches = ( response ).reFindNoCase( "]*>admin", "all" ); + expect( ArrayLen( adminMatches ) ).toBe( 1 ); var openSamlObjectRepresentingResponse = openSamlUtils.xmlToOpenSamlObject( response ); try { From d01121fa7b1b62cd5b9c2c9a5cad83c9dd127203 Mon Sep 17 00:00:00 2001 From: Sacha Moufarrege Date: Wed, 17 Sep 2025 13:59:23 +0200 Subject: [PATCH 3/5] SAML-26: Fix test --- tests/unit/ResponseBuilderTest.cfc | 39 ++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/tests/unit/ResponseBuilderTest.cfc b/tests/unit/ResponseBuilderTest.cfc index e793ff3..609ed04 100644 --- a/tests/unit/ResponseBuilderTest.cfc +++ b/tests/unit/ResponseBuilderTest.cfc @@ -85,11 +85,40 @@ component extends="testbox.system.BaseSpec" { expect( response ).toInclude( "expert" ); expect( response ).toInclude( "admin" ); - // Count the number of AttributeValue elements for roles - var expertMatches = ( response ).reFindNoCase( "]*>expert", "all" ); - expect( ArrayLen( expertMatches ) ).toBe( 1 ); - var adminMatches = ( response ).reFindNoCase( "]*>admin", "all" ); - expect( ArrayLen( adminMatches ) ).toBe( 1 ); + // Parse XML and count AttributeValue elements for roles + var xmlDoc = XmlParse( response ); + var attributeStatements = xmlDoc.xmlRoot.xmlChildren; + var rolesAttribute = ""; + var attributeValues = []; + + // Find the roles attribute in the assertion + for ( var assertion in attributeStatements ) { + if ( assertion.xmlName == "Assertion" ) { + for ( var statement in assertion.xmlChildren ) { + if ( statement.xmlName == "AttributeStatement" ) { + for ( var attribute in statement.xmlChildren ) { + if ( attribute.xmlName == "Attribute" && attribute.xmlAttributes.Name == "roles" ) { + rolesAttribute = attribute; + break; + } + } + } + } + } + } + + expect( rolesAttribute ).notToBeEmpty(); + + // Count AttributeValue elements within the roles attribute + for ( var child in rolesAttribute.xmlChildren ) { + if ( child.xmlName == "AttributeValue" ) { + ArrayAppend( attributeValues, child.xmlText ); + } + } + + expect( ArrayLen( attributeValues ) ).toBe( 2 ); + expect( attributeValues ).toInclude( "expert" ); + expect( attributeValues ).toInclude( "admin" ); var openSamlObjectRepresentingResponse = openSamlUtils.xmlToOpenSamlObject( response ); try { From 1c9be8776e53b662ac4ccf2a1c8ab20649af341e Mon Sep 17 00:00:00 2001 From: Sacha Moufarrege Date: Wed, 17 Sep 2025 14:29:23 +0200 Subject: [PATCH 4/5] SAML-26: Refactor test --- tests/unit/ResponseBuilderTest.cfc | 71 ------------------------------ 1 file changed, 71 deletions(-) diff --git a/tests/unit/ResponseBuilderTest.cfc b/tests/unit/ResponseBuilderTest.cfc index 609ed04..ac016da 100644 --- a/tests/unit/ResponseBuilderTest.cfc +++ b/tests/unit/ResponseBuilderTest.cfc @@ -56,77 +56,6 @@ component extends="testbox.system.BaseSpec" { fail( "SAML did not validate" ); } } ); - - it( "should handle attributes with array values", function(){ - var builder = _getBuilder(); - var response = builder.buildAuthenticationAssertion( - issuer = "http://www.thewebsite.com/" - , nameIdFormat = "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent" - , nameIdValue = "test@test.com" - , inResponseTo = "aaf23196-1773-2113-474a-fe114412ab72" - , recipientUrl = "https://sp.example.com/SAML2/SSO/POST" - , audience = "https://sp.example.com/SAML2" - , sessionTimeout = 30 - , sessionIndex = "C894146D-598F-4D9B-8733ACF80280C4B7" - , attributes = { - email = "test@test.com", - displayName="Test user", - firstName="Test", - lastName="user", - roles = ["expert", "admin"] - } - , privateKey = testPk - , publicCertificate = testCert - ); - - expect( IsXml( response ) ).toBeTrue(); - expect( response ).toInclude( "" ); - expect( response ).toInclude( " Date: Wed, 17 Sep 2025 15:06:00 +0200 Subject: [PATCH 5/5] SAML-26: Add tests --- tests/unit/ResponseBuilderTest.cfc | 41 ++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/tests/unit/ResponseBuilderTest.cfc b/tests/unit/ResponseBuilderTest.cfc index ac016da..61cc8db 100644 --- a/tests/unit/ResponseBuilderTest.cfc +++ b/tests/unit/ResponseBuilderTest.cfc @@ -56,6 +56,47 @@ component extends="testbox.system.BaseSpec" { fail( "SAML did not validate" ); } } ); + + it( "should include one AttributeValue element when attribute is a string with a single value", function(){ + var builder = _getBuilder(); + var response = builder.buildAuthenticationAssertion( + issuer = "http://www.thewebsite.com/" + , nameIdFormat = "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent" + , nameIdValue = "test@test.com" + , inResponseTo = "aaf23196-1773-2113-474a-fe114412ab72" + , recipientUrl = "https://sp.example.com/SAML2/SSO/POST" + , audience = "https://sp.example.com/SAML2" + , sessionTimeout = 30 + , sessionIndex = "C894146D-598F-4D9B-8733ACF80280C4B7" + , attributes = { expertise = "admin" } + , privateKey = testPk + , publicCertificate = testCert + ); + + var matches = ReMatchNoCase( "<(?:[A-Za-z0-9-]+:)?AttributeValue[ >]", response ); + expect( ArrayLen( matches ) ).toBe( 1 ); + } ); + + it( "should include two AttributeValue elements when attribute has multiple values", function(){ + var builder = _getBuilder(); + var response = builder.buildAuthenticationAssertion( + issuer = "http://www.thewebsite.com/" + , nameIdFormat = "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent" + , nameIdValue = "test@test.com" + , inResponseTo = "aaf23196-1773-2113-474a-fe114412ab72" + , recipientUrl = "https://sp.example.com/SAML2/SSO/POST" + , audience = "https://sp.example.com/SAML2" + , sessionTimeout = 30 + , sessionIndex = "C894146D-598F-4D9B-8733ACF80280C4B7" + , attributes = { expertise=[ "admin", "expert" ] } + , privateKey = testPk + , publicCertificate = testCert + ); + + var matches = ReMatchNoCase( "<(?:[A-Za-z0-9-]+:)?AttributeValue[ >]", response ); + expect( ArrayLen( matches ) ).toBe( 2 ); + } ); + } ); describe( "buildErrorResponse()", function(){