Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@
import java.util.Map;
import java.util.function.Consumer;

import static org.cloudfoundry.identity.uaa.util.UaaUrlUtils.normalizeUrlForPortComparison;

/**
* This was copied from Spring Security, and modified to work with Open SAML 4.0.x
* The original class only works with Open SAML 4.1.x+
Expand Down Expand Up @@ -225,7 +227,9 @@ public static Converter<ResponseToken, Saml2ResponseValidatorResult> createDefau
String issuer = response.getIssuer().getValue();
String destination = response.getDestination();
String location = token.getRelyingPartyRegistration().getAssertionConsumerServiceLocation();
if (StringUtils.hasText(destination) && !destination.equals(location)) {
String normalizedDestination = normalizeUrlForPortComparison(destination);
String normalizedLocation = normalizeUrlForPortComparison(location);
if (StringUtils.hasText(destination) && !normalizedDestination.equals(normalizedLocation)) {
String message = "Invalid destination [%s], location [%s] combo for SAML response [%s]"
.formatted(destination, location, response.getID());
result = result.concat(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, message));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,4 +356,31 @@ private static String decodeUriPath(final String path) {

throw new IllegalArgumentException("Aborted url decoding for repeatedly encoded path");
}

/**
* Normalizes a URL for port comparison by removing standard ports (80 for HTTP, 443 for HTTPS).
* This ensures that URLs like "http://example.com" and "http://example.com:80" are treated as equivalent.
*
* @param url the URL to normalize
* @return the normalized URL with standard ports removed, or the original URL if malformed
*/
public static String normalizeUrlForPortComparison(String url) {
if (url == null) {
return null;
}
try {
URI uri = new URI(url);
int port = uri.getPort();
String scheme = uri.getScheme();

if (("http".equalsIgnoreCase(scheme) && port == 80) ||
("https".equalsIgnoreCase(scheme) && port == 443)) {
return new URI(scheme, uri.getUserInfo(), uri.getHost(), -1,
uri.getPath(), uri.getQuery(), uri.getFragment()).toString();
}
} catch (Exception ignored) {
// ignore
}
return url;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,140 @@ void authenticateWhenInvalidDestinationThenThrowAuthenticationException() {
.satisfies(errorOf(Saml2ErrorCodes.INVALID_DESTINATION));
}

@Test
void authenticateWhenDestinationHasStandardHttpPortButLocationDoesNotThenSucceeds() {
// Test scenario where destination includes explicit port 80 but location doesn't
String destinationWithPort = "http://localhost:80/uaa/saml/SSO/alias/integration-saml-entity-id";
String locationWithoutPort = "http://localhost/uaa/saml/SSO/alias/integration-saml-entity-id";

Response response = response(destinationWithPort, ASSERTING_PARTY_ENTITY_ID);
Assertion assertion = assertion();
// Set the recipient in subject confirmation to match the location (without port)
assertion.getSubject().getSubjectConfirmations().forEach(sc ->
sc.getSubjectConfirmationData().setRecipient(locationWithoutPort));
response.getAssertions().add(assertion);

RelyingPartyRegistration.Builder registrationBuilder = verifying(registration())
.assertionConsumerServiceLocation(locationWithoutPort);

Saml2AuthenticationToken token = token(signed(response), registrationBuilder);

// This should not throw an exception due to URL normalization
assertThatNoException().isThrownBy(() -> this.provider.authenticate(token));
}

@Test
void authenticateWhenDestinationHasStandardHttpsPortButLocationDoesNotThenSucceeds() {
// Test scenario where destination includes explicit port 443 but location doesn't
String destinationWithPort = "https://localhost:443/uaa/saml/SSO/alias/integration-saml-entity-id";
String locationWithoutPort = "https://localhost/uaa/saml/SSO/alias/integration-saml-entity-id";

Response response = response(destinationWithPort, ASSERTING_PARTY_ENTITY_ID);
Assertion assertion = assertion();
// Set the recipient in subject confirmation to match the location (without port)
assertion.getSubject().getSubjectConfirmations().forEach(sc ->
sc.getSubjectConfirmationData().setRecipient(locationWithoutPort));
response.getAssertions().add(assertion);

RelyingPartyRegistration.Builder registrationBuilder = verifying(registration())
.assertionConsumerServiceLocation(locationWithoutPort);

Saml2AuthenticationToken token = token(signed(response), registrationBuilder);

// This should not throw an exception due to URL normalization
assertThatNoException().isThrownBy(() -> this.provider.authenticate(token));
}

@Test
void authenticateWhenLocationHasStandardPortButDestinationDoesNotThenSucceeds() {
// Test reverse scenario where location includes explicit port but destination doesn't
String destinationWithoutPort = "http://localhost/uaa/saml/SSO/alias/integration-saml-entity-id";
String locationWithPort = "http://localhost:80/uaa/saml/SSO/alias/integration-saml-entity-id";

Response response = response(destinationWithoutPort, ASSERTING_PARTY_ENTITY_ID);
Assertion assertion = assertion();
// Set the recipient in subject confirmation to match the location (with port)
assertion.getSubject().getSubjectConfirmations().forEach(sc ->
sc.getSubjectConfirmationData().setRecipient(locationWithPort));
response.getAssertions().add(assertion);

RelyingPartyRegistration.Builder registrationBuilder = verifying(registration())
.assertionConsumerServiceLocation(locationWithPort);

Saml2AuthenticationToken token = token(signed(response), registrationBuilder);

// This should not throw an exception due to URL normalization
assertThatNoException().isThrownBy(() -> this.provider.authenticate(token));
}

@Test
void authenticateWhenNonStandardPortMismatchThenThrowsException() {
// Test that non-standard ports still cause validation failure when they don't match
String destinationWithPort8080 = "http://localhost:8080/uaa/saml/SSO/alias/integration-saml-entity-id";
String locationWithPort8081 = "http://localhost:8081/uaa/saml/SSO/alias/integration-saml-entity-id";

Response response = response(destinationWithPort8080, ASSERTING_PARTY_ENTITY_ID);
Assertion assertion = assertion();
// Set the recipient in subject confirmation to match the location (port 8081)
assertion.getSubject().getSubjectConfirmations().forEach(sc ->
sc.getSubjectConfirmationData().setRecipient(locationWithPort8081));
response.getAssertions().add(assertion);

RelyingPartyRegistration.Builder registrationBuilder = verifying(registration())
.assertionConsumerServiceLocation(locationWithPort8081);

Saml2AuthenticationToken token = token(signed(response), registrationBuilder);

// This should still throw an exception since ports don't match and aren't standard
assertThatExceptionOfType(Saml2AuthenticationException.class)
.isThrownBy(() -> this.provider.authenticate(token))
.satisfies(errorOf(Saml2ErrorCodes.INVALID_DESTINATION));
}

@Test
void authenticateWhenDestinationIsEmpty_thenSkipsValidationWithoutNPE() {
// Test that when destination is empty/null, validation is skipped without throwing NPE
String validLocation = DESTINATION;

// Create response with null/empty destination
Response response = response(null, ASSERTING_PARTY_ENTITY_ID);
Assertion assertion = assertion();
assertion.getSubject().getSubjectConfirmations().forEach(sc ->
sc.getSubjectConfirmationData().setRecipient(validLocation));
response.getAssertions().add(assertion);

RelyingPartyRegistration.Builder registrationBuilder = verifying(registration())
.assertionConsumerServiceLocation(validLocation);

Saml2AuthenticationToken token = token(signed(response), registrationBuilder);

// This should not throw a NullPointerException when destination is null
// The validation should be skipped since StringUtils.hasText(destination) returns false
assertThatNoException().isThrownBy(() -> this.provider.authenticate(token));
}

@Test
void authenticateWhenMalformedUrlsButIdentical_thenSucceeds() {
// Test that malformed URLs that can't be normalized still work if they're identical
// This tests the robustness of the comparison when normalization might fail
String malformedUrl = "http://[malformed:url:with:brackets]/saml/SSO/alias/integration-saml-entity-id";

Response response = response(malformedUrl, ASSERTING_PARTY_ENTITY_ID);
Assertion assertion = assertion();
assertion.getSubject().getSubjectConfirmations().forEach(sc ->
sc.getSubjectConfirmationData().setRecipient(malformedUrl));
response.getAssertions().add(assertion);

RelyingPartyRegistration.Builder registrationBuilder = verifying(registration())
.assertionConsumerServiceLocation(malformedUrl);

Saml2AuthenticationToken token = token(signed(response), registrationBuilder);

// Even with malformed URLs that normalization can't handle,
// authentication should succeed if both URLs are identical
assertThatNoException().isThrownBy(() -> this.provider.authenticate(token));
}

@Test
void authenticateWhenNoAssertionsPresentThenThrowAuthenticationException() {
Saml2AuthenticationToken token = token();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType;
import static org.cloudfoundry.identity.uaa.util.UaaUrlUtils.normalizeUrlForPortComparison;
import static org.mockito.Mockito.mock;

@ExtendWith(PollutionPreventionExtension.class)
Expand Down Expand Up @@ -710,4 +711,54 @@ private static Map<String, String> getUnsuccessfulUrls(List<String> urls, boolea
private static List<String> convertToHttps(List<String> urls) {
return urls.stream().map(url -> url.replace("http:", "https:")).toList();
}

// Tests for normalizeUrlForPortComparison method
@ParameterizedTest(name = "normalizeUrlForPortComparison: \"{0}\" should become \"{1}\"")
@CsvSource({
// Standard port removal
"'http://example.com:80/path', 'http://example.com/path'",
"'https://example.com:443/path', 'https://example.com/path'",

// Non-standard ports preserved
"'http://example.com:8080/path', 'http://example.com:8080/path'",
"'https://example.com:8443/path', 'https://example.com:8443/path'",

// URLs without explicit ports remain unchanged
"'http://example.com/path', 'http://example.com/path'",
"'https://example.com/path', 'https://example.com/path'",

// Query parameters and fragments preserved
"'http://example.com:80/path?param1=value1&param2=value2', 'http://example.com/path?param1=value1&param2=value2'",
"'https://example.com:443/path#section1', 'https://example.com/path#section1'",

// Complex URLs with user info
"'http://user:[email protected]:80/path/to/resource?query=value#fragment', 'http://user:[email protected]/path/to/resource?query=value#fragment'",

// Subdomains preserved
"'https://subdomain.example.com:443/path', 'https://subdomain.example.com/path'",

// Different schemes keep non-standard ports
"'ftp://example.com:80/path', 'ftp://example.com:80/path'",

// SAML-specific URL
"'https://uaa.example.com:443/saml/SSO/alias/provider-name?RelayState=https://app.example.com&param=value#section', 'https://uaa.example.com/saml/SSO/alias/provider-name?RelayState=https://app.example.com&param=value#section'",

// Malformed URLs return unchanged
"'not-a-valid-url', 'not-a-valid-url'",
"'http://[invalid:url:with:colons:everywhere', 'http://[invalid:url:with:colons:everywhere'",
"'http://example.com:80/path with spaces and special chars!@#$%^&*()', 'http://example.com:80/path with spaces and special chars!@#$%^&*()'",

// Empty string
"'', ''"
})
void normalizeUrlForPortComparison_parameterizedTests(String inputUrl, String expectedUrl) {
String result = normalizeUrlForPortComparison(inputUrl);
assertThat(result).isNotNull().isEqualTo(expectedUrl); // Ensure we never return null for non-null input
}

@Test
void normalizeUrlForPortComparison_withNullUrl_returnsNull() {
String result = normalizeUrlForPortComparison(null);
assertThat(result).isNull();
}
}
Loading