api7 / api7/lua-resty-saml

The issuer allow-list is not weighed again when a session resumes

Open
#48 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. 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.

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 by reading login at lua/resty/saml.lua:196 and compare its session-resume path with login_callback's issuer allow-list check. Trace how issuer and idp_cert are stored in sessions, including sessions predating the option. Done requires a maintainer-approved policy for resumed sessions and coverage for both existing and newly established sessions.

Written by the indexing model from the issue text.

Assessment

Tech stack
lua
Domain
authentication, backend, security
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.