Skip to content

refactor(cas-auth): drop unnecessary raw Cookie header fallback#13635

Open
shreemaan-abhishek wants to merge 1 commit into
apache:masterfrom
shreemaan-abhishek:fix/cas-auth-remove-cookie-fallback
Open

refactor(cas-auth): drop unnecessary raw Cookie header fallback#13635
shreemaan-abhishek wants to merge 1 commit into
apache:masterfrom
shreemaan-abhishek:fix/cas-auth-remove-cookie-fallback

Conversation

@shreemaan-abhishek

Copy link
Copy Markdown
Contributor

Description

The get_cookie helper in cas-auth was added to fall back to parsing the raw Cookie header when $cookie_<name> returned nil, on the theory that nginx's $cookie_<name> variable doesn't reliably expose cookies with long names (the per-config session cookie is CAS_SESSION_<sha256-hex>, 76 chars).

That fallback isn't needed: nginx exposes cookies via $cookie_<name> regardless of name length on supported OpenResty builds. This drops the helper and reads the session cookie directly via ctx.var["cookie_" .. opts.cookie_name], consistent with how the CAS_REQUEST_URI cookie is already read in the same plugin.

No behavior change on supported runtimes; pure cleanup of dead code.

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 (existing cas-auth tests cover cookie reads; no new behavior)
  • I have updated the documentation to reflect this change (internal helper, no doc impact)
  • I have verified that this change passes lint

Read the per-config session cookie directly via $cookie_<name>, the same
way the CAS_REQUEST_URI cookie is already read. nginx reliably exposes
cookies by name regardless of length, so the manual http_cookie parsing
fallback in get_cookie was dead weight.
@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. tech debt labels Jun 30, 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

Removes the get_cookie helper from the cas-auth plugin and reads the per-config CAS session cookie directly via ctx.var["cookie_" .. opts.cookie_name], aligning this access pattern with the existing CAS_REQUEST_URI cookie handling in the same plugin.

Changes:

  • Deleted the get_cookie(ctx, name) fallback implementation that parsed the raw Cookie header.
  • Updated session-cookie reads in logout() and _M.access() to use ctx.var["cookie_" .. opts.cookie_name] directly.

💡 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

size:S This PR changes 10-29 lines, ignoring generated files. tech debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants