[TNZ-27070]: refactor tests after review

Author: joemahady-comm

server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/OpenSaml4AuthenticationProviderUnitTests.java

@@ -228,6 +228,50 @@ void authenticateWhenNonStandardPortMismatchThenThrowsException() {
                 .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();

server/src/test/java/org/cloudfoundry/identity/uaa/util/UaaUrlUtilsTest.java

@@ -713,148 +713,53 @@ private static List<String> convertToHttps(List<String> urls) {
     }
 
     // Tests for normalizeUrlForPortComparison method
-    @Test
-    void normalizeUrlForPortComparison_withHttpStandardPort_removesPort() {
-        String urlWithPort = "http://example.com:80/path";
-        String expected = "http://example.com/path";
-        
-        String result = normalizeUrlForPortComparison(urlWithPort);
-        
-        assertThat(result).isEqualTo(expected);
-    }
-
-    @Test
-    void normalizeUrlForPortComparison_withHttpsStandardPort_removesPort() {
-        String urlWithPort = "https://example.com:443/path";
-        String expected = "https://example.com/path";
-        
-        String result = normalizeUrlForPortComparison(urlWithPort);
-        
-        assertThat(result).isEqualTo(expected);
-    }
-
-    @Test
-    void normalizeUrlForPortComparison_withHttpNonStandardPort_keepsPort() {
-        String urlWithPort = "http://example.com:8080/path";
-        String expected = "http://example.com:8080/path";
-        
-        String result = normalizeUrlForPortComparison(urlWithPort);
-        
-        assertThat(result).isEqualTo(expected);
-    }
-
-    @Test
-    void normalizeUrlForPortComparison_withHttpsNonStandardPort_keepsPort() {
-        String urlWithPort = "https://example.com:8443/path";
-        String expected = "https://example.com:8443/path";
-        
-        String result = normalizeUrlForPortComparison(urlWithPort);
-        
-        assertThat(result).isEqualTo(expected);
-    }
-
-    @Test
-    void normalizeUrlForPortComparison_withNoPort_remainsUnchanged() {
-        String urlWithoutPort = "http://example.com/path";
-        String expected = "http://example.com/path";
-        
-        String result = normalizeUrlForPortComparison(urlWithoutPort);
-        
-        assertThat(result).isEqualTo(expected);
-    }
-
-    @Test
-    void normalizeUrlForPortComparison_withHttpsNoPort_remainsUnchanged() {
-        String urlWithoutPort = "https://example.com/path";
-        String expected = "https://example.com/path";
-        
-        String result = normalizeUrlForPortComparison(urlWithoutPort);
-        
-        assertThat(result).isEqualTo(expected);
-    }
-
-    @Test
-    void normalizeUrlForPortComparison_withQueryParams_preservesQueryParams() {
-        String urlWithPort = "http://example.com:80/path?param1=value1&param2=value2";
-        String expected = "http://example.com/path?param1=value1&param2=value2";
-        
-        String result = normalizeUrlForPortComparison(urlWithPort);
-        
-        assertThat(result).isEqualTo(expected);
-    }
-
-    @Test
-    void normalizeUrlForPortComparison_withFragment_preservesFragment() {
-        String urlWithPort = "https://example.com:443/path#section1";
-        String expected = "https://example.com/path#section1";
-        
-        String result = normalizeUrlForPortComparison(urlWithPort);
-        
-        assertThat(result).isEqualTo(expected);
-    }
-
-    @Test
-    void normalizeUrlForPortComparison_withMalformedUrl_returnsOriginal() {
-        String malformedUrl = "not-a-valid-url";
-        
-        String result = normalizeUrlForPortComparison(malformedUrl);
-        
-        assertThat(result).isEqualTo(malformedUrl);
+    @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).isEqualTo(expectedUrl);
+        assertThat(result).isNotNull(); // Ensure we never return null for non-null input
     }
 
     @Test
     void normalizeUrlForPortComparison_withNullUrl_returnsNull() {
         String result = normalizeUrlForPortComparison(null);
-        
         assertThat(result).isNull();
     }
-
-    @Test
-    void normalizeUrlForPortComparison_withEmptyUrl_returnsEmpty() {
-        String emptyUrl = "";
-        
-        String result = normalizeUrlForPortComparison(emptyUrl);
-        
-        assertThat(result).isEqualTo(emptyUrl);
-    }
-
-    @Test
-    void normalizeUrlForPortComparison_withComplexUrl_handlesCorrectly() {
-        String complexUrl = "http://user:[email protected]:80/path/to/resource?query=value#fragment";
-        String expected = "http://user:[email protected]/path/to/resource?query=value#fragment";
-        
-        String result = normalizeUrlForPortComparison(complexUrl);
-        
-        assertThat(result).isEqualTo(expected);
-    }
-
-    @Test
-    void normalizeUrlForPortComparison_withSubdomain_preservesSubdomain() {
-        String urlWithSubdomain = "https://subdomain.example.com:443/path";
-        String expected = "https://subdomain.example.com/path";
-        
-        String result = normalizeUrlForPortComparison(urlWithSubdomain);
-        
-        assertThat(result).isEqualTo(expected);
-    }
-
-    @Test
-    void normalizeUrlForPortComparison_withDifferentScheme_keepsPort() {
-        String urlWithPort = "ftp://example.com:80/path";
-        String expected = "ftp://example.com:80/path";
-        
-        String result = normalizeUrlForPortComparison(urlWithPort);
-        
-        assertThat(result).isEqualTo(expected);
-    }
-
-    @Test
-    void normalizeUrlForPortComparison_withSamlSpecificUrl_handlesCorrectly() {
-        String samlUrl = "https://uaa.example.com:443/saml/SSO/alias/provider-name?RelayState=https://app.example.com&param=value#section";
-        String expected = "https://uaa.example.com/saml/SSO/alias/provider-name?RelayState=https://app.example.com&param=value#section";
-        
-        String result = normalizeUrlForPortComparison(samlUrl);
-        
-        assertThat(result).isEqualTo(expected);
-    }
 }