feat: per-request AssertionConsumerServiceIndex for createLoginRequest (closes #437)#623
Merged
Conversation
closes #437) - saml-core §3.4.1 — `<samlp:AuthnRequest>` schema: `AssertionConsumerServiceIndex`, `AssertionConsumerServiceURL`, and `ProtocolBinding` are all `use="optional"` and mutually exclusive (the spec text: "If the `<AssertionConsumerServiceIndex>` attribute is present, neither `<AssertionConsumerServiceURL>` nor `<ProtocolBinding>` may be set"). - saml-profiles §4.1.4.1 — Web Browser SSO Profile permits the index form; the IdP looks up the endpoint in the SP's metadata. Some IdPs (legacy Shibboleth deployments, certain ADFS configurations) prefer the metadata-indexed ACS form so the SP can declare multiple ACS endpoints once and select one by index per request, rather than echoing the URL+ProtocolBinding pair in every AuthnRequest. samlify previously hardcoded `ProtocolBinding="…HTTP-POST"` and only emitted the URL form, which left those peers unaddressable from this library. - `CreateLoginRequestOptions.assertionConsumerServiceIndex?: number` — per-request opt-in. JSDoc spells out mutual exclusion semantics: when set, the URL and ProtocolBinding are both omitted (index wins). - `defaultLoginRequestTemplate` now uses placeholders for all three attributes; the omission rule from #614 (`replaceTagsByValue` drops attributes whose value is `null`/`undefined`) handles the swap so no new omission logic is added. - `entity-sp.ts:createLoginRequest` plumbs the new option to all three binding builders (`redirect`, `post`, `simpleSign`), mirroring the shape introduced for `forceAuthn` in #618. - Each binding builder accepts `assertionConsumerServiceIndex?: number` and sets `ProtocolBinding` / `AssertionConsumerServiceURL` to `undefined` when the index is supplied; the metadata-derived ACS URL is suppressed in that branch. - New `describe('AssertionConsumerServiceIndex (#437, saml-core §3.4.1)')` block in `test/units.ts` covering: - Default template renders the index attribute and drops URL + ProtocolBinding when the index is set (incl. `index=0`). - Default template renders URL + ProtocolBinding (and drops the index) when the index is undefined. - Byte-identical pin: literal-string compare on the rendered XML for a backwards-compatible legacy call (modulo `ID=` and `IssueInstant=`). - Redirect / post / simpleSign bindings each verified end-to-end on the real default-template path. - Backwards-compat regression for each binding when no options are passed. - `yarn test` (302 / 1 skipped) and `yarn coverage` (≥90 across all metrics) pass; `npx tsc` is clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
assertionConsumerServiceIndex?: numbertoCreateLoginRequestOptionsso SP callers can issue<samlp:AuthnRequest AssertionConsumerServiceIndex="N" ...>instead of the URL + ProtocolBinding pair, closing Does samlify supports using AssertionConsumerServiceIndex instead of ProtocolBinding and AssertionConsumerServiceURL? #437.AssertionConsumerServiceURLandProtocolBindingare both omitted from the rendered request (mutual exclusion per saml-core §3.4.1). The metadata-derived ACS URL the SP would otherwise inject is also suppressed; the index wins.ID=andIssueInstant=).Spec reference
saml-core §3.4.1—<samlp:AuthnRequest>schema:AssertionConsumerServiceIndex,AssertionConsumerServiceURL, andProtocolBindingare alluse="optional"and mutually exclusive ("If the<AssertionConsumerServiceIndex>attribute is present, neither<AssertionConsumerServiceURL>nor<ProtocolBinding>may be set").saml-profiles §4.1.4.1— Web Browser SSO Profile permits the index form; the IdP resolves the endpoint from the SP's metadata.Implementation notes
defaultLoginRequestTemplatenow uses placeholders for all three attributes. The omission rule shipped in fix: omit AuthnRequest attributes whose value is null/undefined (closes #455) #614 (attribute is dropped when its value isnull/undefined) handles the swap, so no new omission logic was introduced.entity-sp.ts:createLoginRequestextracts the new option (mirroring theforceAuthnshape from feat: per-request ForceAuthn for createLoginRequest (closes #359) #618) and forwards it tobinding-redirect.ts,binding-post.ts, andbinding-simplesign.ts, each of which setsProtocolBinding/AssertionConsumerServiceURLtoundefinedwhen the index is supplied.Test plan
AssertionConsumerServiceIndexand dropsAssertionConsumerServiceURL+ProtocolBindingwhen the index is set.AssertionConsumerServiceURL+ProtocolBindingand drops the index whenAssertionConsumerServiceIndexis undefined (regression for legacy default behaviour).ID=/IssueInstant=).sp.createLoginRequest(idp, 'redirect', { assertionConsumerServiceIndex: 1 })— inflated SAMLRequest containsAssertionConsumerServiceIndex="1", no URL, no ProtocolBinding.'post'and'simpleSign'bindings.index=0edge case — falsy but defined; the attribute still renders.ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"andAssertionConsumerServiceURL=.yarn test— 302 passing / 1 skipped.yarn coverage— ≥90 % on every metric (statements 97.33, branches 90.87, functions 99.15, lines 97.33).npx tsc— clean.🤖 Generated with Claude Code