api7 / api7/lua-resty-saml

idp_issuers is enforced on login but not on logout

Open
#49 1 comment 1 reaction 0 assignees View on GitHub

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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.