Skip to content

Conversation

@joemahady-comm
Copy link
Contributor

Fix UAA on standard ports with mismatching Location and Destination

@strehle strehle requested a review from Copilot September 15, 2025 09:36
Copy link
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

This PR fixes UAA SAML authentication issues when dealing with standard ports by implementing URL normalization. The fix ensures that URLs with explicitly specified standard ports (80 for HTTP, 443 for HTTPS) are treated as equivalent to their counterparts without explicit port numbers during destination validation.

Key changes:

  • Added URL normalization functionality to handle standard port equivalence
  • Updated SAML authentication provider to use normalized URLs for destination validation
  • Added comprehensive test coverage for the new normalization functionality

Reviewed Changes

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

File Description
UaaUrlUtils.java Added normalizeUrlForPortComparison method to remove standard ports from URLs
OpenSaml4AuthenticationProvider.java Integrated URL normalization into SAML destination validation logic
UaaUrlUtilsTest.java Added comprehensive test suite for the new URL normalization method
OpenSaml4AuthenticationProviderUnitTests.java Added integration tests for SAML authentication with standard port scenarios

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

strehle
strehle previously approved these changes Sep 15, 2025
Copy link
Member

@strehle strehle left a comment

Choose a reason for hiding this comment

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

beside one remark from sonar

https://sonarcloud.io/summary/new_code?id=cloudfoundry-identity-parent&pullRequest=3621

I dont have further things, so thanks for the PR

@github-project-automation github-project-automation bot moved this from Inbox to Pending Merge | Prioritized in Foundational Infrastructure Working Group Sep 15, 2025
@strehle strehle requested review from duanemay and fhanik September 15, 2025 09:38
strehle
strehle previously approved these changes Sep 15, 2025
Copy link
Member

@strehle strehle left a comment

Choose a reason for hiding this comment

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

Copy link
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 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@fhanik fhanik left a comment

Choose a reason for hiding this comment

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

Excellent. Glad we got this back in.

Copy link
Member

@duanemay duanemay left a comment

Choose a reason for hiding this comment

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

Awesome!

@duanemay duanemay merged commit 1485642 into cloudfoundry:develop Sep 15, 2025
31 of 32 checks passed
@github-project-automation github-project-automation bot moved this from Pending Merge | Prioritized to Done in Foundational Infrastructure Working Group Sep 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

4 participants