From ba965061cb213d8897a8575c95d2ad944671d2eb Mon Sep 17 00:00:00 2001 From: Matteo Mara Date: Mon, 26 Sep 2022 14:01:18 +0200 Subject: [PATCH] SONAR-17296 sanitize SAML response fields --- .../saml/SamlAuthStatusPageGenerator.java | 27 +-- .../src/main/resources/samlAuthResult.html | 20 +- .../saml/SamlAuthStatusPageGeneratorTest.java | 23 +- .../resources/samlAuthResultComplete.html | 209 ------------------ .../test/resources/samlAuthResultEmpty.html | 20 +- 5 files changed, 29 insertions(+), 270 deletions(-) delete mode 100644 server/sonar-auth-saml/src/test/resources/samlAuthResultComplete.html diff --git a/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlAuthStatusPageGenerator.java b/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlAuthStatusPageGenerator.java index 82a3a8ae035..c6093ebd0a5 100644 --- a/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlAuthStatusPageGenerator.java +++ b/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlAuthStatusPageGenerator.java @@ -23,20 +23,14 @@ import com.google.common.io.Resources; import java.io.IOException; import java.net.URL; import java.nio.charset.StandardCharsets; -import java.util.List; +import java.util.Base64; import java.util.Map; -import java.util.stream.Collectors; -import javax.annotation.Nullable; import org.json.JSONObject; public final class SamlAuthStatusPageGenerator { private static final String WEB_CONTEXT = "%WEB_CONTEXT%"; - private static final String STATUS = "%STATUS%"; - private static final String ERRORS = "%ERRORS%"; - private static final String WARNINGS = "%WARNINGS%"; - private static final String AVAILABLE_ATTRIBUTES = "%AVAILABLE_ATTRIBUTES%"; - private static final String ATTRIBUTE_MAPPINGS = "%ATTRIBUTE_MAPPINGS%"; + private static final String SAML_AUTHENTICATION_STATUS = "%SAML_AUTHENTICATION_STATUS%"; private static final String HTML_TEMPLATE_NAME = "samlAuthResult.html"; @@ -57,21 +51,12 @@ public final class SamlAuthStatusPageGenerator { private static Map getSubstitutionsMap(SamlAuthenticationStatus samlAuthenticationStatus) { return Map.of( WEB_CONTEXT, "", - STATUS, toJsString(samlAuthenticationStatus.getStatus()), - ERRORS, toJsArrayFromList(samlAuthenticationStatus.getErrors()), - WARNINGS, toJsArrayFromList(samlAuthenticationStatus.getWarnings()), - AVAILABLE_ATTRIBUTES, new JSONObject(samlAuthenticationStatus.getAvailableAttributes()).toString(), - ATTRIBUTE_MAPPINGS, new JSONObject(samlAuthenticationStatus.getMappedAttributes()).toString()); + SAML_AUTHENTICATION_STATUS, getBase64EncodedStatus(samlAuthenticationStatus)); } - private static String toJsString(@Nullable String inputString) { - return String.format("'%s'", inputString != null ? inputString.replace("'", "\\'") : ""); - } - - private static String toJsArrayFromList(List inputArray) { - return "[" + inputArray.stream() - .map(SamlAuthStatusPageGenerator::toJsString) - .collect(Collectors.joining(",")) + "]"; + private static String getBase64EncodedStatus(SamlAuthenticationStatus samlAuthenticationStatus) { + byte[] bytes = new JSONObject(samlAuthenticationStatus).toString().getBytes(StandardCharsets.UTF_8); + return String.format("'%s'", Base64.getEncoder().encodeToString(bytes)); } private static String getPlainTemplate() { diff --git a/server/sonar-auth-saml/src/main/resources/samlAuthResult.html b/server/sonar-auth-saml/src/main/resources/samlAuthResult.html index 398266b2d29..85e987b33ca 100644 --- a/server/sonar-auth-saml/src/main/resources/samlAuthResult.html +++ b/server/sonar-auth-saml/src/main/resources/samlAuthResult.html @@ -124,7 +124,7 @@ function createSectionTitle(title) { const node = document.createElement("h2"); - node.innerText = title; + node.textContent = title; return node; } @@ -134,7 +134,7 @@ arr.forEach((item) => { const message = document.createElement("li"); message.className = className; - message.innerText = item; + message.textContent = item; list.appendChild(message); }); @@ -150,7 +150,7 @@ const row = document.createElement("tr"); const keyNode = document.createElement("td"); - keyNode.innerText = key; + keyNode.textContent = key; row.appendChild(keyNode); const valueNode = document.createElement("td"); @@ -173,16 +173,18 @@ container.appendChild(box); } - const status = %STATUS%; - const attributes = %AVAILABLE_ATTRIBUTES%; - const mappings = %ATTRIBUTE_MAPPINGS%; - const errors = %ERRORS%; - const warnings = %WARNINGS%; + const response = %SAML_AUTHENTICATION_STATUS%; + const decodedStatus = JSON.parse(atob(response)); + const status = decodedStatus.status; + const attributes = decodedStatus.availableAttributes; + const mappings = decodedStatus.mappedAttributes; + const errors = decodedStatus.errors; + const warnings = decodedStatus.warnings; // Switch status class const statusNode = document.querySelector("#status"); statusNode.classList.add(status); - statusNode.innerText = status; + statusNode.textContent = status; // generate content const container = document.querySelector("#content"); diff --git a/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlAuthStatusPageGeneratorTest.java b/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlAuthStatusPageGeneratorTest.java index 966d61d440f..7e5e43953c2 100644 --- a/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlAuthStatusPageGeneratorTest.java +++ b/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlAuthStatusPageGeneratorTest.java @@ -25,8 +25,6 @@ import java.net.URL; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.HashMap; -import java.util.List; -import java.util.Map; import org.junit.Test; import static org.junit.Assert.assertEquals; @@ -35,30 +33,11 @@ import static org.mockito.Mockito.when; import static org.sonar.auth.saml.SamlAuthStatusPageGenerator.getSamlAuthStatusHtml; public class SamlAuthStatusPageGeneratorTest { - - private final SamlAuthenticationStatus samlAuthenticationStatus = mock(SamlAuthenticationStatus.class); - - private static final String HTML_TEMPLATE_NAME = "samlAuthResultComplete.html"; private static final String EMPTY_HTML_TEMPLATE_NAME = "samlAuthResultEmpty.html"; - @Test - public void test_full_html_generation() { - - when(samlAuthenticationStatus.getStatus()).thenReturn("success"); - when(samlAuthenticationStatus.getErrors()).thenReturn(List.of("error1", "error2 'with message'")); - when(samlAuthenticationStatus.getWarnings()).thenReturn(List.of("warning1", "warning2 'with message'")); - when(samlAuthenticationStatus.getAvailableAttributes()).thenReturn(Map.of("key1", List.of("value1", "value2 with weird chars \n\t\"\\"), "key2", List.of("value3", "value4"))); - when(samlAuthenticationStatus.getMappedAttributes()).thenReturn(Map.of("key1", List.of("value1", "value2"), "key2", List.of("value3", "value4"))); - - String completeHtmlTemplate = getSamlAuthStatusHtml(samlAuthenticationStatus); - String expectedTemplate = loadTemplateFromResources(HTML_TEMPLATE_NAME); - - assertEquals(expectedTemplate, completeHtmlTemplate); - - } - @Test public void test_full_html_generation_with_empty_values() { + SamlAuthenticationStatus samlAuthenticationStatus = mock(SamlAuthenticationStatus.class); when(samlAuthenticationStatus.getStatus()).thenReturn(null); when(samlAuthenticationStatus.getErrors()).thenReturn(new ArrayList<>()); diff --git a/server/sonar-auth-saml/src/test/resources/samlAuthResultComplete.html b/server/sonar-auth-saml/src/test/resources/samlAuthResultComplete.html deleted file mode 100644 index f12552ec361..00000000000 --- a/server/sonar-auth-saml/src/test/resources/samlAuthResultComplete.html +++ /dev/null @@ -1,209 +0,0 @@ - - - - - - - - - - - - - - - - - - - - SAML Authentication Test - - - - - -
-

SAML Authentication Test

-
-
-
-
- - - - diff --git a/server/sonar-auth-saml/src/test/resources/samlAuthResultEmpty.html b/server/sonar-auth-saml/src/test/resources/samlAuthResultEmpty.html index 6ae670910bd..de4879d8ce3 100644 --- a/server/sonar-auth-saml/src/test/resources/samlAuthResultEmpty.html +++ b/server/sonar-auth-saml/src/test/resources/samlAuthResultEmpty.html @@ -124,7 +124,7 @@ function createSectionTitle(title) { const node = document.createElement("h2"); - node.innerText = title; + node.textContent = title; return node; } @@ -134,7 +134,7 @@ arr.forEach((item) => { const message = document.createElement("li"); message.className = className; - message.innerText = item; + message.textContent = item; list.appendChild(message); }); @@ -150,7 +150,7 @@ const row = document.createElement("tr"); const keyNode = document.createElement("td"); - keyNode.innerText = key; + keyNode.textContent = key; row.appendChild(keyNode); const valueNode = document.createElement("td"); @@ -173,16 +173,18 @@ container.appendChild(box); } - const status = ''; - const attributes = {}; - const mappings = {}; - const errors = []; - const warnings = []; + const response = 'eyJ3YXJuaW5ncyI6W10sImF2YWlsYWJsZUF0dHJpYnV0ZXMiOnt9LCJlcnJvcnMiOltdLCJtYXBwZWRBdHRyaWJ1dGVzIjp7fX0='; + const decodedStatus = JSON.parse(atob(response)); + const status = decodedStatus.status; + const attributes = decodedStatus.availableAttributes; + const mappings = decodedStatus.mappedAttributes; + const errors = decodedStatus.errors; + const warnings = decodedStatus.warnings; // Switch status class const statusNode = document.querySelector("#status"); statusNode.classList.add(status); - statusNode.innerText = status; + statusNode.textContent = status; // generate content const container = document.querySelector("#content"); -- 2.39.5