feat: expose the IdP's claim set and the Subject NameID

changelog.md

@@ -9,6 +9,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- `ISSOAuthorizationResponse.getClaims()` and `getClaim( name, defaultValue )` expose everything the IdP
+  asserted, keyed by the name the IdP used - the WS-Federation claim URIs for SAML, the id token or user
+  info keys for oAuth. The typed getters are a lowest common denominator of the four providers, so an
+  Entra group or role claim, a Google `hd`, or a customer's employee-number claim had nowhere to go and no
+  way to be read: a consumer had to re-parse `getRawResponseData()` itself. One map means the interface
+  does not grow a getter per claim, and it reads the same way for a SAML attribute as for an oAuth claim.
+- `ISSOAuthorizationResponse.getNameId()` and `getNameIdFormat()` expose the Subject's NameID, which no
+  attribute can substitute for and which the response could not reach at all. The Format comes with it
+  because it decides what the value means: Entra's default is a pairwise identifier scoped to one app
+  registration, so the same person arrives under a different NameID at a second registration in the same
+  tenant. Treat one as an identifier without reading the Format and you have keyed identity to a value
+  that is not portable.
+- `SAMLParsingService.extractUserInfo()` returns `claims`, `nameId` and `nameIdFormat` alongside the
+  existing fields. A claim always holds an array, since a SAML attribute may carry several
+  AttributeValues - Entra's `authnmethodsreferences` and its group claims do - and an IdP may split one
+  claim across repeated `Attribute` elements. Values are trimmed, which pretty-printed assertions need.
+- `MicrosoftSAMLProvider` sets the claims on the success path only. An assertion whose signature did not
+  verify has asserted nothing, so a consumer reading a claim off a failed response would be trusting
+  whoever sent it rather than the IdP.
+
 ### Changed
 
 - **BREAKING** `SSOAuthorizationResponse.getName()` returned `FirstName` instead of `Name`, so the value
@@ -21,6 +43,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- `SAMLParsingService` matched `//Attribute[@Name='...']`, which only resolves when the assertion carries
+  the SAML namespace as its default - `extractUserInfo()` strips default namespace declarations, and
+  nothing else. An IdP that prefixes its elements, as ADFS and Shibboleth do and Entra can be configured
+  to, therefore yielded no first name, surname or object identifier, and the whole response was reported
+  as `Failed to extract user information`. The typed fields are now derived from the claim set, which is
+  matched on `local-name()`.
+
 - [#16](https://github.com/coldbox-modules/cbSSO/issues/16) An unregistered provider name threw a
   `KeyNotFoundException` from `ProviderService.get()` before the handler's `isNull()` guard could run,
   so `CBSSOMissingProvider` was never announced from `Auth.start()` or `Auth.authorize()`. The

models/ISSOAuthorizationResponse.cfc

@@ -9,5 +9,9 @@ interface {
 	public string function getLastName();
 	public any function getRawResponseData();
 	public string function getErrorMessage();
+	public struct function getClaims();
+	public string function getClaim( required string name, string defaultValue );
+	public string function getNameId();
+	public string function getNameIdFormat();
 
 }

models/SSOAuthorizationResponse.cfc

@@ -9,6 +9,9 @@ component implements="cbsso.models.ISSOAuthorizationResponse" accessors=true {
 	property name="LastName";
 	property name="RawResponseData";
 	property name="ErrorMessage";
+	property name="Claims";
+	property name="NameId";
+	property name="NameIdFormat";
 
 	/**
 	 * Seeds every property, so a response that only ever had its failure fields populated still
@@ -23,6 +26,9 @@ component implements="cbsso.models.ISSOAuthorizationResponse" accessors=true {
 		variables.LastName        = "";
 		variables.ErrorMessage    = "";
 		variables.RawResponseData = {};
+		variables.Claims          = {};
+		variables.NameId          = "";
+		variables.NameIdFormat    = "";
 
 		return this;
 	}
@@ -82,4 +88,78 @@ component implements="cbsso.models.ISSOAuthorizationResponse" accessors=true {
 		return variables.ErrorMessage;
 	}
 
+	/**
+	 * Everything the IdP asserted, keyed by the name it used - the WS-Federation claim URIs for SAML, the
+	 * id token or user info keys for oAuth. The typed getters above cover what every provider has in
+	 * common; this is where anything else lives, so reaching a group, role or employee-number claim does
+	 * not need a getter of its own.
+	 */
+	public struct function getClaims(){
+		return variables.Claims;
+	}
+
+	/**
+	 * The first value of a claim, which is what a caller wants in all but the multi-valued case. Struct
+	 * keys are case-insensitive, so the name does not have to match the IdP's casing.
+	 */
+	public string function getClaim( required string name, string defaultValue = "" ){
+		if ( !variables.Claims.keyExists( arguments.name ) || !variables.Claims[ arguments.name ].len() ) {
+			return arguments.defaultValue;
+		}
+
+		return variables.Claims[ arguments.name ][ 1 ];
+	}
+
+	/**
+	 * Normalised here rather than in each provider, so `getClaims()` reads the same way whatever produced
+	 * it: every claim holds an array, because a SAML attribute and an oAuth claim can both be
+	 * multi-valued. Values that are not simple - a nested object in an id token - are left out, and stay
+	 * reachable on `getRawResponseData()`.
+	 */
+	public any function setClaims( required struct claims ){
+		var normalised = {};
+
+		for ( var name in arguments.claims ) {
+			var value = arguments.claims[ name ];
+
+			if ( isSimpleValue( value ) ) {
+				normalised[ name ] = [ toString( value ) ];
+				continue;
+			}
+
+			if ( !isArray( value ) ) {
+				continue;
+			}
+
+			normalised[ name ] = [];
+
+			for ( var entry in value ) {
+				if ( isSimpleValue( entry ) ) {
+					normalised[ name ].append( toString( entry ) );
+				}
+			}
+		}
+
+		variables.Claims = normalised;
+
+		return this;
+	}
+
+	/**
+	 * The Subject's NameID, which SAML always carries and no claim can substitute for. Empty for oAuth
+	 * providers, and for a SAML assertion that identifies its subject by attribute alone.
+	 */
+	public string function getNameId(){
+		return variables.NameId;
+	}
+
+	/**
+	 * The NameID's Format. Read it before treating a NameID as an identifier: Entra's default is a
+	 * pairwise value scoped to one app registration, so the same person arrives under a different NameID
+	 * at a second registration in the same tenant.
+	 */
+	public string function getNameIdFormat(){
+		return variables.NameIdFormat;
+	}
+
 }

models/providers/FacebookProvider.cfc

@@ -74,6 +74,7 @@ component
 				.setLastName( idTokenData.family_name )
 				.setEmail( idTokenData.email )
 				.setUserId( idTokenData.sub )
+				.setClaims( idTokenData )
 		} catch ( any e ) {
 			return authResponse.setWasSuccessful( false ).setErrorMessage( e.message );
 		}

models/providers/GitHubProvider.cfc

@@ -77,7 +77,8 @@ component
 				.setWasSuccessful( true )
 				.setName( userData.name )
 				.setEmail( userData.email )
-				.setUserId( userData.id );
+				.setUserId( userData.id )
+				.setClaims( userData );
 		} catch ( any e ) {
 			return authResponse.setWasSuccessful( false ).setErrorMessage( e.message );
 		}

models/providers/GoogleProvider.cfc

@@ -69,6 +69,7 @@ component
 				.setLastName( idTokenData.family_name )
 				.setEmail( idTokenData.email )
 				.setUserId( idTokenData.sub )
+				.setClaims( idTokenData )
 		} catch ( any e ) {
 			return authResponse.setWasSuccessful( false ).setErrorMessage( e.message );
 		}

models/providers/MicrosoftSAMLProvider.cfc

@@ -73,12 +73,17 @@ component
 					.setErrorMessage( samlData.errorMessage );
 			}
 
+			// Set only here, not on the failure returns above: an assertion whose signature did not verify
+			// has asserted nothing, and a consumer reading a claim off it would be trusting the sender.
 			return authResponse
 				.setWasSuccessful( true )
 				.setFirstName( samlData.firstName )
 				.setLastName( samlData.lastName )
 				.setEmail( samlData.email )
 				.setUserId( samlData.userId )
+				.setClaims( samlData.claims )
+				.setNameId( samlData.nameId )
+				.setNameIdFormat( samlData.nameIdFormat )
 				.setRawResponseData( data );
 		} catch ( any e ) {
 			return authResponse.setWasSuccessful( false ).setErrorMessage( e.message );

models/utility/SAMLParsingService.cfc

@@ -1,5 +1,17 @@
 component singleton {
 
+	/**
+	 * The WS-Federation and Microsoft claim URIs the typed fields are derived from. Every other attribute
+	 * the IdP asserted is reachable through `claims`, under the name the IdP used.
+	 */
+	variables.claimNames = {
+		"givenName"        : "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/givenname",
+		"surname"          : "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/surname",
+		"name"             : "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/name",
+		"emailAddress"     : "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress",
+		"objectIdentifier" : "http://schemas.microsoft.com/identity/claims/objectidentifier"
+	};
+
 	public struct function extractUserInfo( required string rawSAMLResponse ){
 		var data = {
 			"success"      : false,
@@ -8,7 +20,10 @@ component singleton {
 			"firstName"    : "",
 			"lastName"     : "",
 			"email"        : "",
-			"userId"       : ""
+			"userId"       : "",
+			"nameId"       : "",
+			"nameIdFormat" : "",
+			"claims"       : {}
 		};
 		var xmlData = xmlParse( rawSAMLResponse.reReplace( "xmlns="".+?""", "", "all" ) );
 
@@ -21,10 +36,18 @@ component singleton {
 			}
 
 			try {
-				data.firstName = extractFirstName( xmlData );
-				data.lastName  = extractLastName( xmlData );
-				data.email     = extractEmail( xmlData );
-				data.userId    = extractUserId( xmlData );
+				var subject = extractSubjectNameId( xmlData );
+
+				// Populated before the required claims are read, so a response that fails on a missing
+				// one still reports what the IdP actually asserted.
+				data.claims       = extractClaims( xmlData );
+				data.nameId       = subject.value;
+				data.nameIdFormat = subject.format;
+
+				data.firstName = requiredClaim( data.claims, variables.claimNames.givenName );
+				data.lastName  = requiredClaim( data.claims, variables.claimNames.surname );
+				data.email     = extractEmail( data.claims );
+				data.userId    = requiredClaim( data.claims, variables.claimNames.objectIdentifier );
 
 				return data;
 			} catch ( any e ) {
@@ -42,13 +65,23 @@ component singleton {
 		return data;
 	}
 
+	/**
+	 * Matched on local-name() rather than the `samlp:` prefix. extractUserInfo() strips only the default
+	 * namespace declaration, so `xmlns:samlp` survives on the document - but BoxLang's xmlSearch does not
+	 * resolve a prefixed XPath against a prefix declared in the document, so `//samlp:StatusCode` finds
+	 * nothing there and a valid, signed, successful assertion is reported as a failure. local-name() is
+	 * the form that behaves the same on every engine.
+	 */
 	private boolean function detectSuccess( required xmlDoc ){
-		return xmlSearch( xmlDoc, "//samlp:StatusCode[@Value='urn:oasis:names:tc:SAML:2.0:status:Success']" ).len() == 1;
+		return xmlSearch(
+			xmlDoc,
+			"//*[local-name()='StatusCode' and @Value='urn:oasis:names:tc:SAML:2.0:status:Success']"
+		).len() == 1;
 	}
 
 	private string function extractErrorMessage( required xmlDoc ){
 		try {
-			return xmlSearch( xmlDoc, "//samlp:StatusMessage" )[ 1 ].xmlchildren[ 1 ].xmltext;
+			return xmlSearch( xmlDoc, "//*[local-name()='StatusMessage']" )[ 1 ].xmlchildren[ 1 ].xmltext;
 		} catch ( any e ) {
 			try {
 				var nodes = xmlSearch( xmlDoc, "//*" );
@@ -64,47 +97,82 @@ component singleton {
 		}
 	}
 
-	private string function extractFirstName( required xmlDoc ){
-		return xmlSearch(
-			xmlDoc,
-			"//Attribute[@Name='http://schemas.xmlsoap.org/ws/2005/05/identity/claims/givenname']"
-		)[ 1 ].xmlchildren[ 1 ].xmltext;
-	}
+	/**
+	 * Every asserted attribute, keyed by its `Name` and always holding an array - a claim may carry more
+	 * than one AttributeValue (Entra group and role claims routinely do), and an IdP may split one claim
+	 * across repeated Attribute elements.
+	 */
+	private struct function extractClaims( required xmlDoc ){
+		var claims = {};
 
-	private string function extractLastName( required xmlDoc ){
-		return xmlSearch(
-			xmlDoc,
-			"//Attribute[@Name='http://schemas.xmlsoap.org/ws/2005/05/identity/claims/surname']"
-		)[ 1 ].xmlchildren[ 1 ].xmltext;
-	}
+		for ( var node in xmlSearch( xmlDoc, "//*[local-name()='Attribute'][@Name]" ) ) {
+			var name = trim( node.xmlAttributes.Name );
 
-	private string function extractEmail( required xmlDoc ){
-		// try emailAddress claim first, then fallback to name claim if emailAddress is not present
-		var emailNodes = xmlSearch(
-			xmlDoc,
-			"//Attribute[@Name='http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress']"
-		);
+			if ( !len( name ) ) {
+				continue;
+			}
+
+			if ( !claims.keyExists( name ) ) {
+				claims[ name ] = [];
+			}
 
-		if ( arrayLen( emailNodes ) > 0 ) {
-			return emailNodes[ 1 ].xmlchildren[ 1 ].xmltext;
+			for ( var valueNode in node.xmlChildren ) {
+				if ( listLast( valueNode.xmlName, ":" ) == "AttributeValue" ) {
+					claims[ name ].append( trim( valueNode.xmlText ) );
+				}
+			}
 		}
 
-		var nameNodes = xmlSearch(
-			xmlDoc,
-			"//Attribute[@Name='http://schemas.xmlsoap.org/ws/2005/05/identity/claims/name']"
-		);
-		if ( arrayLen( nameNodes ) > 0 ) {
-			return nameNodes[ 1 ].xmlchildren[ 1 ].xmltext;
+		return claims;
+	}
+
+	/**
+	 * The Format matters as much as the value: Entra's default is a pairwise identifier scoped to the app
+	 * registration, stable within that registration and meaningless outside it. A consumer cannot tell a
+	 * portable identifier from a scoped one without it.
+	 */
+	private struct function extractSubjectNameId( required xmlDoc ){
+		var nodes = xmlSearch( xmlDoc, "//*[local-name()='Subject']/*[local-name()='NameID']" );
+
+		if ( !nodes.len() ) {
+			return { "value" : "", "format" : "" };
 		}
 
-		return "";
+		var attributes = nodes[ 1 ].xmlAttributes;
+
+		return {
+			"value"  : trim( nodes[ 1 ].xmlText ),
+			"format" : attributes.keyExists( "Format" ) ? trim( attributes.Format ) : ""
+		};
 	}
 
-	private string function extractUserId( required xmlDoc ){
-		return xmlSearch(
-			xmlDoc,
-			"//Attribute[@Name='http://schemas.microsoft.com/identity/claims/objectidentifier']"
-		)[ 1 ].xmlchildren[ 1 ].xmltext;
+	/**
+	 * Falls back to the `name` claim, which carries the UPN when no email claim is mapped.
+	 */
+	private string function extractEmail( required struct claims ){
+		var email = claimValue( claims, variables.claimNames.emailAddress );
+
+		return len( email ) ? email : claimValue( claims, variables.claimNames.name );
+	}
+
+	private string function claimValue( required struct claims, required string name ){
+		return claims.keyExists( name ) && claims[ name ].len() ? claims[ name ][ 1 ] : "";
+	}
+
+	/**
+	 * Still throws when the claim is absent, so an assertion missing one of the values the typed fields
+	 * are built from fails exactly as it did before the claim set was exposed. Whether a missing
+	 * display-name claim should fail a login at all is a separate question from reaching the claims.
+	 */
+	private string function requiredClaim( required struct claims, required string name ){
+		if ( !claims.keyExists( name ) ) {
+			throw(
+				type    = "SAMLParsingService.MissingClaim",
+				message = "The assertion contains no '#name#' claim."
+			);
+		}
+
+		return claimValue( claims, name );
 	}
 
 }