Skip to content

Fix for Issue #3650#3894

Merged
strehle merged 2 commits into
developfrom
fix/issue/3650/saml-redirect
May 6, 2026
Merged

Fix for Issue #3650#3894
strehle merged 2 commits into
developfrom
fix/issue/3650/saml-redirect

Conversation

@strehle
Copy link
Copy Markdown
Member

@strehle strehle commented May 4, 2026

Problem: When a SAML IDP's metadata endpoint is unreachable, the ConfiguratorRelyingPartyRegistrationRepository catches the exception, returns null, and the DelegatingRelyingPartyRegistrationRepository falls through to the DefaultRelyingPartyRegistrationRepository which returns a stub registration
pointing to https://www.cloudfoundry.org. The user gets redirected there with all SAML request parameters — confusing UX and a data leak.

  Problem: When a SAML IDP's metadata endpoint is unreachable, the ConfiguratorRelyingPartyRegistrationRepository catches the exception, returns null, and the DelegatingRelyingPartyRegistrationRepository falls through to the DefaultRelyingPartyRegistrationRepository which returns a stub registration
  pointing to https://www.cloudfoundry.org. The user gets redirected there with all SAML request parameters — confusing UX and a data leak.
@strehle strehle linked an issue May 4, 2026 that may be closed by this pull request
@strehle strehle requested a review from Copilot May 4, 2026 08:48
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Prevents a security/UX failure mode in the SAML login flow where failures to fetch IdP metadata could previously fall through to the default stub relying party registration (sending users to https://www.cloudfoundry.org with SAML request parameters).

Changes:

  • Fail fast in ConfiguratorRelyingPartyRegistrationRepository by throwing a Saml2Exception on registration build errors (instead of returning null and allowing fallback).
  • Improve /error500 handling to surface SAML failures as external_auth_error for both direct Saml2Exception and “cause is Saml2Exception” cases.
  • Update/add tests to assert the new exception behavior and error page rendering.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/ConfiguratorRelyingPartyRegistrationRepository.java Throws Saml2Exception on metadata/build failures to prevent fallback to the default stub registration.
server/src/main/java/org/cloudfoundry/identity/uaa/home/HomeController.java Extracts Saml2Exception (direct or cause) to return external_auth_error with 400 status.
server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/ConfiguratorRelyingPartyRegistrationRepositoryTest.java Updates tests to expect Saml2Exception for invalid metadata inputs.
server/src/test/java/org/cloudfoundry/identity/uaa/login/HomeControllerViewTests.java Adds coverage for a direct Saml2Exception passed to /error500.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 4 out of 4 changed files in this pull request and generated 3 comments.

Comment thread server/src/main/java/org/cloudfoundry/identity/uaa/home/HomeController.java Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 4 out of 4 changed files in this pull request and generated no new comments.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 4 out of 4 changed files in this pull request and generated no new comments.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 4 out of 4 changed files in this pull request and generated no new comments.

@github-project-automation github-project-automation Bot moved this from Inbox to Pending Merge | Prioritized in Foundational Infrastructure Working Group May 5, 2026
@fhanik fhanik added the accepted Accepted the issue label May 5, 2026
@strehle strehle merged commit 1de2ffe into develop May 6, 2026
46 of 47 checks passed
@strehle strehle deleted the fix/issue/3650/saml-redirect branch May 6, 2026 04:39
@github-project-automation github-project-automation Bot moved this from Pending Merge | Prioritized to Done in Foundational Infrastructure Working Group May 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accepted Accepted the issue

Projects

Development

Successfully merging this pull request may close these issues.

UAA can send you to cloudfoundry.org instead of your IDP

4 participants