From 8aec375fa6b589bbbe2229cf3b8c64f5e0601226 Mon Sep 17 00:00:00 2001
From: Matteo Mara <matteo.mara@sonarsource.com>
Date: Mon, 20 Mar 2023 12:53:43 +0100
Subject: [PATCH] SONAR-18805 improve validation of the SAML authentication
 when login or name is empty

---
 .../sonar/auth/saml/SamlStatusChecker.java    | 14 +++++++++++++-
 .../auth/saml/SamlStatusCheckerTest.java      | 19 ++++++++++++++++++-
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlStatusChecker.java b/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlStatusChecker.java
index 4ed1f7967da..ec96d2aaa0b 100644
--- a/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlStatusChecker.java
+++ b/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlStatusChecker.java
@@ -113,7 +113,12 @@ public final class SamlStatusChecker {
       USER_NAME_ATTRIBUTE, samlSettings.getUserName(),
       USER_LOGIN_ATTRIBUTE, samlSettings.getUserLogin());
 
-    return generateMissingMappingMessages(mappings, auth);
+    List<String> mappingErrors = generateMissingMappingMessages(mappings, auth);
+    if (mappingErrors.isEmpty()) {
+      mappingErrors = generateEmptyMappingsMessages(mappings, auth);
+    }
+
+    return mappingErrors;
   }
 
   private static List<String> generateMissingMappingMessages(Map<String, String> mappings, Auth auth) {
@@ -124,4 +129,11 @@ public final class SamlStatusChecker {
       .toList();
   }
 
+  private static List<String> generateEmptyMappingsMessages(Map<String, String> mappings, Auth auth) {
+    return mappings.entrySet()
+      .stream()
+      .filter(entry -> (auth.getAttribute(entry.getValue()).size() == 1 && auth.getAttribute(entry.getValue()).contains("")))
+      .map(entry -> String.format("Mapping found for the property %s, but the field %s is empty in the SAML response.", entry.getKey(), entry.getValue()))
+      .toList();
+  }
 }
diff --git a/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlStatusCheckerTest.java b/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlStatusCheckerTest.java
index f7e23f3edd4..74c93afe70b 100644
--- a/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlStatusCheckerTest.java
+++ b/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlStatusCheckerTest.java
@@ -59,7 +59,7 @@ public class SamlStatusCheckerTest {
 
   @Before
   public void setUp() {
-    when(auth.getErrors()).thenReturn(new ArrayList<String>());
+    when(auth.getErrors()).thenReturn(new ArrayList<>());
     when(auth.getSettings()).thenReturn(new Saml2Settings());
     when(auth.getAttributes()).thenReturn(getResponseAttributes());
   }
@@ -158,6 +158,23 @@ public class SamlStatusCheckerTest {
       .contains(String.format("Mapping not found for the property %s, the field %s is not available in the SAML response.", USER_NAME_ATTRIBUTE, "wrongNameField")));
   }
 
+  @Test
+  public void authentication_has_errors_when_login_and_name_are_empty() {
+    setSettings();
+    when(auth.getAttributes()).thenReturn(getEmptyAttributes());
+    getEmptyAttributes().forEach((key, value) -> when(auth.getAttribute(key)).thenReturn(value));
+
+    samlAuthenticationStatus = getSamlAuthenticationStatus(auth, new SamlSettings(settings.asConfig()));
+
+    assertEquals("error", samlAuthenticationStatus.getStatus());
+    assertTrue(samlAuthenticationStatus.getWarnings().isEmpty());
+    assertEquals(2, samlAuthenticationStatus.getErrors().size());
+    assertTrue(samlAuthenticationStatus.getErrors()
+      .contains(String.format("Mapping found for the property %s, but the field %s is empty in the SAML response.", USER_LOGIN_ATTRIBUTE, "login")));
+    assertTrue(samlAuthenticationStatus.getErrors()
+      .contains(String.format("Mapping found for the property %s, but the field %s is empty in the SAML response.", USER_NAME_ATTRIBUTE, "name")));
+  }
+
   @Test
   public void authentication_has_no_warnings_when_optional_mappings_are_not_provided() {
     setSettings();
-- 
2.39.5