Skip to content

fix(cas-auth): stop SLO callback POST from being proxied upstream#13610

Open
shreemaan-abhishek wants to merge 1 commit into
apache:masterfrom
shreemaan-abhishek:fix/cas-auth-slo-post-fallthrough
Open

fix(cas-auth): stop SLO callback POST from being proxied upstream#13610
shreemaan-abhishek wants to merge 1 commit into
apache:masterfrom
shreemaan-abhishek:fix/cas-auth-slo-post-fallthrough

Conversation

@shreemaan-abhishek

Copy link
Copy Markdown
Contributor

Description

The cas-auth plugin protects routes and also captures the configured cas_callback_uri so it can handle the IdP's single-logout (SLO) POST callback.

In _M.access, the SLO POST branch parses the SAML SessionIndex, optionally deletes the matching session, and then falls through the function. Because the branch has no terminating return, _M.access returns nil, which APISIX treats as "continue the phase chain", so the IdP's logout POST is proxied to the upstream unauthenticated.

This patch ends the SLO branch with an explicit return ngx.HTTP_OK after the logout bookkeeping, so the callback POST is fully handled by the plugin and never reaches the upstream.

Fixes

  • apisix/plugins/cas-auth.lua: terminate the SLO POST branch with 200.
  • t/plugin/cas-auth.t: regression test sending a well-formed SLO POST to a route whose upstream is a closed port; it must return 200 from the plugin rather than a 502 from a fall-through proxy attempt.
  • docs/en/latest/plugins/cas-auth.md: document that SLO POST callbacks are handled by the plugin and not forwarded upstream.

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible

A POST to cas_callback_uri carrying a SAML SessionIndex handled the
single-logout bookkeeping and then fell through _M.access, returning nil.
APISIX treats a nil access result as 'continue', so the IdP logout POST
was proxied to the upstream unauthenticated. Terminate the branch with
an explicit 200 after handling SLO, and add a regression test asserting
the callback POST never reaches the upstream.
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Jun 26, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a control-flow bug in the cas-auth plugin where a CAS IdP single-logout (SLO) POST to cas_callback_uri could be unintentionally proxied to the upstream due to the SLO branch falling through the access handler. The change ensures the plugin fully terminates the SLO callback request and never forwards it upstream.

Changes:

  • Terminate the SLO POST callback path in cas-auth with an explicit return ngx.HTTP_OK.
  • Add a regression test that would fail with a 502 if the request were still being proxied to a closed upstream port.
  • Document that SLO POST callbacks are handled by the plugin and not forwarded upstream.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
apisix/plugins/cas-auth.lua Adds an explicit return ngx.HTTP_OK to stop SLO callback POST requests from falling through to upstream proxying.
t/plugin/cas-auth.t Adds a regression test route with a closed-port upstream and asserts SLO POST returns 200 from the plugin (not 502).
docs/en/latest/plugins/cas-auth.md Documents that SLO POST callbacks to cas_callback_uri are handled by the plugin and not forwarded upstream.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants