idp_issuers is enforced on login but not on logout
Nobody has claimed this yet.
- Dominant language
- Perl
- Stars
- 2
- Forks
- 3
- Avg merge
- 2d 20h
- Merged PRs (30d)
- 3
Description
Raised on #41, and only became possible with the change in that PR.
What
idp_issuers gates the login callback. The logout callback reads an issuer too, at lua/resty/saml.lua:443, compares it against the one on the session and only warns:
if issuer ~= saved_issuer then
ngx.log(ngx.WARN, "issuer different: issuer=", issuer, ", data.issuer=", saved_issuer)
end
It then destroys the session whatever the outcome. So of the two paths where a message arrives from outside, the pin is enforced on one.
Why it is worth doing now rather than before
Until #41 a LogoutRequest could carry its identity in a message the signature never covered, so pinning its issuer would have compared text the sender typed. That PR makes such a message fail verification, so the value is now covered by the signature and a comparison against it means something.
Shape
Weigh the issuer of a logout message against idp_issuers the way the login callback does, and decide what the existing session comparison should be: a warning that precedes an unconditional sess:destroy() is either a check or it is not.
Impact is bounded either way, since the outcome is the caller's own session being destroyed. It matters more for anything downstream that consumes doc_name_id from a logout message to drive its own session store.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in lua/resty/saml.lua:443 and compare the logout issuer handling with the login callback's idp_issuers enforcement. Review the existing session issuer comparison and decide whether a mismatch must prevent destruction or only be logged. Done means the logout path consistently enforces the chosen issuer checks for signed logout messages.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- lua
- Domain
- authentication, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100