Skip to content

Fix CSP header omitting for versioned SAML endpoints - #5288

Merged
supersven merged 6 commits into
developfrom
sventennie/fix-csp-for-versioned-saml-endpoints
Jun 25, 2026
Merged

Fix CSP header omitting for versioned SAML endpoints#5288
supersven merged 6 commits into
developfrom
sventennie/fix-csp-for-versioned-saml-endpoints

Conversation

@supersven

Copy link
Copy Markdown
Contributor

The prior URL regex was intended to cover SAML endpoints, but did not cover the version path segment.

The expected behaviour is that - in the multi-ingress case - we omit CSP headers for the SAML endpoints. This implicitly resembles the non-multi-ingress case.

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

The prior URL regex was intended to cover SAML endpoints, but did not
cover the version path segment.

The expected behaviour is that - in the multi-ingress case - we omit CSP
headers for the SAML endpoints. This implicitly resembles the
non-multi-ingress case.
@supersven
supersven requested a review from Copilot June 24, 2026 09:01
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Jun 24, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates the multi-ingress CSP injection logic so that CSP headers continue to be omitted for SAML SSO endpoints even when those endpoints are accessed through a versioned API prefix (e.g. /v15/sso/...). This aligns multi-ingress behavior with the non-multi-ingress setup and avoids breaking SAML flows.

Changes:

  • Extend the nginx configuration-snippet URI regex to also match versioned SAML endpoints (/v[0-9]+/sso/...) and therefore skip CSP injection for them.
  • Add clarifying inline comments explaining why SAML endpoints must be excluded from CSP header setting.
  • Add a changelog entry documenting the fix.

Reviewed changes

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

File Description
charts/nginx-ingress-services/templates/ingress.yaml Updates the CSP-skip regex to include optional /vN prefix for SAML endpoints in multi-ingress.
changelog.d/3-bug-fixes/multi-ingress-saml-csp Documents the bug fix for versioned SAML endpoints receiving CSP headers.

@supersven
supersven marked this pull request as ready for review June 24, 2026 09:09
@supersven
supersven requested review from a team as code owners June 24, 2026 09:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread charts/nginx-ingress-services/templates/ingress.yaml Outdated
Comment thread charts/nginx-ingress-services/templates/ingress.yaml Outdated
supersven and others added 2 commits June 24, 2026 17:32
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@supersven
supersven merged commit 8adb4d1 into develop Jun 25, 2026
10 checks passed
@supersven
supersven deleted the sventennie/fix-csp-for-versioned-saml-endpoints branch June 25, 2026 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants