Raised on #41. The allow-list added there is checked when a login response arrives and never again.
What
login_callback refuses a response whose issuers are not on idp_issuers, but login (lua/resty/saml.lua:196) returns the stored identity for an existing session, issuer = sess:get("issuer") included, without weighing it against the list.
Why it matters
This is the incident the option exists for. An operator learns that an issuer the shared idp_cert signs for is not one they trust, adds it to the allow-list, and reloads. Every session established before that change keeps working until it expires. Sessions are cookies with no server-side store, so there is nothing to evict per session.
There is a blunt remedy today: rotating secret invalidates all of them at once. Sessions established before the option existed are the reason this is not a one-line change.
The part that needs thought
Sessions predating the change carry no stored issuer, and nil cannot be told apart from an issuer that is no longer allowed:
- refusing on nil logs out every existing session on upgrade
- allowing nil through fails open and hands the incident back
Options worth weighing: store a marker alongside the issuer so the two cases are distinguishable, or make the resume check opt-in, or tie it to a session version that a config change bumps.
Whichever way it goes, the same question applies to idp_cert itself, which a resumed session also never rechecks.
Raised on #41. The allow-list added there is checked when a login response arrives and never again.
What
login_callbackrefuses a response whose issuers are not onidp_issuers, butlogin(lua/resty/saml.lua:196) returns the stored identity for an existing session,issuer = sess:get("issuer")included, without weighing it against the list.Why it matters
This is the incident the option exists for. An operator learns that an issuer the shared
idp_certsigns for is not one they trust, adds it to the allow-list, and reloads. Every session established before that change keeps working until it expires. Sessions are cookies with no server-side store, so there is nothing to evict per session.There is a blunt remedy today: rotating
secretinvalidates all of them at once. Sessions established before the option existed are the reason this is not a one-line change.The part that needs thought
Sessions predating the change carry no stored issuer, and nil cannot be told apart from an issuer that is no longer allowed:
Options worth weighing: store a marker alongside the issuer so the two cases are distinguishable, or make the resume check opt-in, or tie it to a session version that a config change bumps.
Whichever way it goes, the same question applies to
idp_certitself, which a resumed session also never rechecks.