Repository navigation
fix: SAML spec follow-up from the #99 review (5.4.1) - #105
Merged
Merged
Conversation
- /recipe/saml/user: clientId is required - document acsURLs (PUT request, client object, login/callback error cases) with the core versions it exists in; allowIDPInitiatedLogin requires it - version notes for pre-GHSA-3r7x / pre-GHSA-wfg2 5.4 cores, the 12.0 email fallbacks and the 11.x login URL query-string bug - note the 500 on undecodable samlResponse - CHANGELOG: sp-metadata and legacy routes are intentionally out of the CDI Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SamDP1AS8ybBhCc1CWBWWk
This was referenced Oct 28, 2026
Merged
porcellus
marked this pull request as ready for review
October 28, 2026 19:02
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #99 (merged before the review landed) — 5.4.1
Applies Tamás's review on #99:
POST /recipe/saml/user:clientIdis required (core parses it withnullable=false); I had it wrong in cb32ab7.acsURLsis now documented in 5.4 (decision: ADR-007's "no spec hasacsURLs" premise no longer holds, sinceallowIDPInitiatedLogin: trueis a 400 without it on every current core): optional allow-list on the PUT request (non-empty, non-blank strings), required whenallowIDPInitiatedLoginis true, present on the client object when set,INVALID_CLIENT_ERRORon login for anacsURLoutside the list,IDP_LOGIN_DISALLOWED_ERRORon the callback for an IdP-initiated login without a list. Marked "since core 11.3.8 / 11.4.8 / 12.0.11 / 12.1.2 / 12.2.1 / 12.3.1". 5.8 keeps only what it adds on top (nullclears, required-once-set).INVALID_CLIENT_ERRORinstead of a 500 for an unknown issuer on IdP-initiated responses, claims from the first (audience-validated) assertion only (GHSA-wfg2-vqfg-rqh6), the 12.0mail/emailfallbacks, and the 11.x?-vs-&login URL bug.samlResponsethat is not valid base64: documented as a 500 on current cores (worth a core fix to mapIllegalArgumentExceptionto 400; not done here).GET /.well-known/sp-metadataand/recipe/saml/legacy/*are intentionally outside the CDI.After merging: merge
5.4forward into5.5(and on to5.6/5.7/5.8); the later PRs don't duplicate these changes.