Skip to content

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

Open
dougcain wants to merge 1 commit into
coldbox-modules:developmentfrom
dougcain:feat/expose-claims-and-nameid
Open

feat: expose the IdP's claim set and the Subject NameID#20
dougcain wants to merge 1 commit into
coldbox-modules:developmentfrom
dougcain:feat/expose-claims-and-nameid

Conversation

@dougcain

@dougcain dougcain commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Why

ISSOAuthorizationResponse's typed getters are the intersection of what the four providers have in common — email, a name, an id. Anything else an IdP asserts has nowhere to go: an Entra group or role claim, a Google hd, a customer's employee-number claim. A consumer that wants one has to re-parse getRawResponseData() itself, re-implementing the namespace handling this module already does.

Adding a getter per claim would grow the interface without end, so this adds a map instead.

What

response.getClaims()                          // { "<claim name>" : [ "value", ... ] }
response.getClaim( "hd" )                     // first value, or ""
response.getClaim( "employeeNumber", "n/a" )  // ...or the caller's default
response.getNameId()                          // the Subject's NameID
response.getNameIdFormat()                    // ...and what it means

Four methods, closed to further growth, and they read the same way for a SAML attribute as for an oAuth id token claim — which is what makes it worth putting on the shared interface rather than on one provider. MicrosoftSAMLProvider fills the map from the assertion's AttributeStatement; GoogleProvider and FacebookProvider from the verified id token; GitHubProvider from the user info response.

A claim always holds an array. A SAML attribute may carry several AttributeValues — Entra's authnmethodsreferences does, and its group claims do — and an IdP may split one claim across repeated Attribute elements. The existing extraction takes the first value of the first element, so the rest were unreachable and nothing said so. Values that are not simple (a nested object in an id token) are left out of the map and stay on getRawResponseData().

NameID gets its own getters rather than a slot in the map, because it is not an attribute and its Format changes 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. A caller that cannot see the Format cannot tell a portable identifier from a scoped one. The response could not reach NameID at all before this.

Claims are set 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.

Also fixes prefixed assertions

Deriving the typed fields from the claim set fixes them for prefixed assertions as a side effect. The old extractors matched //Attribute[@Name='...'], which resolves only when the assertion carries the SAML namespace as its defaultextractUserInfo() strips default namespace declarations and nothing else. An IdP that prefixes its elements, as ADFS and Shibboleth do and Entra can be configured to, yielded no first name, surname or object identifier, and the whole response came back as Failed to extract user information. The claim walk matches on local-name(), the same lesson as the samlp: prefixes in #19.

New fixture prefixedSAMLResponse.xml covers it.

Deliberately not changed

  • A missing givenname, surname or objectidentifier claim still fails the whole response, as it does today. That looks wrong — a missing display-name claim is a poor reason to refuse a login — but it is a question about required claims, not about reaching them, so it is left alone here. There is a spec pinning the current behaviour, including that the claim set is still reported on that path so you can see what the IdP actually sent.
  • The map is keyed by Name only, not FriendlyName. Shibboleth's urn:oid: names are unusable without a lookup and FriendlyName is the human-readable key, so this may be worth adding — but two ways to reach one claim is ambiguity, and it seemed better asked than assumed.

Notes

  • Additive. Nothing existing changes shape, so this is a minor. It does add four methods to ISSOAuthorizationResponse, which is technically breaking for anyone implementing that interface outside the module — SSOAuthorizationResponse is the only implementation here.
  • Branched off development, so it is independent of fix: restore OpenSAML class loading and make SAML parsing engine-portable #19. The two touch neighbouring hunks of SAMLParsingService.cfc; whichever merges second wants a trivial rebase, and I am happy to do it.

Testing

29 specs pass on both CI engines — BoxLang 1.17.0-snapshot and Lucee 5.4.8.2 — with format:check clean. New coverage: the full claim set from the existing Entra fixture, a multi-valued claim, NameID and Format, a prefixed assertion end to end, the missing-required-claim path, and the normalisation and defaulting rules on the response object.

The typed getters on ISSOAuthorizationResponse are the intersection of what four providers have in
common - email, a name, an id. Anything else an IdP asserts has nowhere to go: an Entra group or role
claim, a Google `hd`, a customer's employee-number claim. A consumer wanting one had to re-parse
getRawResponseData() itself, which means re-implementing the namespace handling this module already
does.

Adding a getter per claim would grow the interface without end, so this adds a map instead:
getClaims() keyed by the name the IdP used, and getClaim( name, default ) for the single-value case.
Closed to further growth, and it reads the same way for a SAML attribute as for an oAuth id token
claim, which is what makes it worth putting on the shared interface rather than on one provider.

A claim always holds an array. A SAML attribute may carry several AttributeValues - Entra's
authnmethodsreferences does, and its group claims do - and an IdP may split one claim across repeated
Attribute elements. The existing extraction took the first value of the first element, so the rest
were unreachable and nothing said so. Values that are not simple, a nested object in an id token,
are left out of the map and stay on getRawResponseData().

NameID gets its own pair of getters rather than a slot in the map, because it is not an attribute and
its Format changes 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. A caller that cannot see the Format cannot tell a portable identifier from a scoped one.
The response could not reach NameID at all before this.

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.

Deriving the typed fields from the claim set also fixes them for prefixed assertions. The old
extractors matched //Attribute[@name='...'], which resolves only 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, yielded
no first name, surname or object identifier, and the response came back as "Failed to extract user
information". Covered by a new prefixed fixture.

A missing givenname, surname or objectidentifier claim still fails the whole response, as it did
before. That looks wrong - a missing display-name claim is a poor reason to refuse a login - but it
is a question about required claims, not about reaching them, so it is left alone here.

29 specs pass on both CI engines: BoxLang 1.17.0-snapshot and Lucee 5.4.8.2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant