fix: stop requiring claims SAML 2.0 does not require

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
@@ -18,9 +40,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   will now receive a full name.
 - `Auth` resolves the requested provider in a `preHandler`, which stores it in `prc.ssoProvider` for
   every action instead of each action resolving it for itself.
+- **BREAKING** `SAMLParsingService` treated a missing `givenname`, `surname` or `objectidentifier` claim as
+  a fatal error, failing the whole response. SAML 2.0 requires none of them: an `<AttributeStatement>` is
+  optional throughout Core, the Web Browser SSO profile (Profiles 4.1.4.2) asks only for a `<Subject>`
+  carrying a bearer `<SubjectConfirmation>`, and all three names are WS-Federation or Microsoft URIs that
+  only an Entra-shaped IdP asserts - so a conformant assertion from ADFS, Shibboleth or Okta was refused
+  over a display name, while the identifier the profile does point at went unread. The display names are
+  now optional, and the subject is identified by the `objectidentifier` claim where the IdP asserts one and
+  by the Subject's NameID where it does not. `firstName` and `lastName` may now be empty on a successful
+  response, where before they were either populated or the response failed.
+- A transient NameID is not accepted as a subject identifier, and an assertion carrying no other is
+  refused with `SAMLParsingService.NoSubjectIdentifier`. The specification defines a transient identifier
+  as valid for a single session, so keying identity to one enrols the same person again on every login.
+  Whether the identifier that *is* returned is portable remains the caller's to judge from `nameIdFormat`:
+  Entra's persistent NameID is pairwise, scoped to one app registration.
 
 ### 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 );