Escape pipe character for injected users (#5175) (#5192)

Signed-off-by: shikharj05 <[email protected]>

Author: shikharj05

src/main/java/org/opensearch/security/privileges/PrivilegesEvaluatorImpl.java

@@ -111,6 +111,7 @@
 
 import static org.opensearch.security.OpenSearchSecurityPlugin.traceAction;
 import static org.opensearch.security.support.ConfigConstants.OPENDISTRO_SECURITY_USER_INFO_THREAD_CONTEXT;
+import static org.opensearch.security.support.SecurityUtils.escapePipe;
 
 public class PrivilegesEvaluatorImpl implements PrivilegesEvaluator {
 
@@ -275,12 +276,14 @@ public boolean isInitialized() {
     private void setUserInfoInThreadContext(User user) {
         if (threadContext.getTransient(OPENDISTRO_SECURITY_USER_INFO_THREAD_CONTEXT) == null) {
             StringJoiner joiner = new StringJoiner("|");
-            joiner.add(user.getName());
-            joiner.add(String.join(",", user.getRoles()));
-            joiner.add(String.join(",", user.getSecurityRoles()));
+            // Escape any pipe characters in the values before joining
+            joiner.add(escapePipe(user.getName()));
+            joiner.add(escapePipe(String.join(",", user.getRoles())));
+            joiner.add(escapePipe(String.join(",", user.getSecurityRoles())));
+
             String requestedTenant = user.getRequestedTenant();
             if (!Strings.isNullOrEmpty(requestedTenant)) {
-                joiner.add(requestedTenant);
+                joiner.add(escapePipe(requestedTenant));
             }
             threadContext.putTransient(OPENDISTRO_SECURITY_USER_INFO_THREAD_CONTEXT, joiner.toString());
         }

src/main/java/org/opensearch/security/privileges/legacy/PrivilegesEvaluatorImpl.java

@@ -109,6 +109,7 @@
 
 import static org.opensearch.security.OpenSearchSecurityPlugin.traceAction;
 import static org.opensearch.security.support.ConfigConstants.OPENDISTRO_SECURITY_USER_INFO_THREAD_CONTEXT;
+import static org.opensearch.security.support.SecurityUtils.escapePipe;
 
 public class PrivilegesEvaluatorImpl implements PrivilegesEvaluator {
 
@@ -245,12 +246,14 @@ public boolean isInitialized() {
     private void setUserInfoInThreadContext(User user) {
         if (threadContext.getTransient(OPENDISTRO_SECURITY_USER_INFO_THREAD_CONTEXT) == null) {
             StringJoiner joiner = new StringJoiner("|");
-            joiner.add(user.getName());
-            joiner.add(String.join(",", user.getRoles()));
-            joiner.add(String.join(",", user.getSecurityRoles()));
+            // Escape any pipe characters in the values before joining
+            joiner.add(escapePipe(user.getName()));
+            joiner.add(escapePipe(String.join(",", user.getRoles())));
+            joiner.add(escapePipe(String.join(",", user.getSecurityRoles())));
+
             String requestedTenant = user.getRequestedTenant();
             if (!Strings.isNullOrEmpty(requestedTenant)) {
-                joiner.add(requestedTenant);
+                joiner.add(escapePipe(requestedTenant));
             }
             threadContext.putTransient(OPENDISTRO_SECURITY_USER_INFO_THREAD_CONTEXT, joiner.toString());
         }

src/main/java/org/opensearch/security/support/SecurityUtils.java

@@ -161,4 +161,12 @@ private static String resolveEnvVar(String envVarName, String mode, boolean bc,
             return bc ? Hasher.hash(envVarValue.toCharArray(), settings) : envVarValue;
         }
     }
+
+    // Helper method to escape pipe characters
+    public static String escapePipe(String input) {
+        if (input == null) {
+            return "";
+        }
+        return input.replace("|", "\\|");
+    }
 }

src/test/java/org/opensearch/security/support/SecurityUtilsTest.java

@@ -70,4 +70,35 @@ private void checkKeysWithPredicate(Collection<String> keys, String predicateNam
             );
         });
     }
+
+    @Test
+    public void testEscapePipe_NullInput() {
+        assertThat("Null input should return empty string", SecurityUtils.escapePipe(null), equalTo(""));
+    }
+
+    @Test
+    public void testEscapePipe_EmptyString() {
+        assertThat("Empty string should return empty string", SecurityUtils.escapePipe(""), equalTo(""));
+    }
+
+    @Test
+    public void testEscapePipe_StringWithoutPipe() {
+        assertThat("String without pipe should remain unchanged", SecurityUtils.escapePipe("normal string"), equalTo("normal string"));
+    }
+
+    @Test
+    public void testEscapePipe_SinglePipe() {
+        assertThat("Single pipe should be escaped", SecurityUtils.escapePipe("before|after"), equalTo("before\\|after"));
+    }
+
+    @Test
+    public void testEscapePipe_MultiplePipes() {
+        assertThat("Multiple pipes should all be escaped", SecurityUtils.escapePipe("a|b|c"), equalTo("a\\|b\\|c"));
+    }
+
+    @Test
+    public void testEscapePipe_MultiplePipesConsecutive() {
+        assertThat("Consecutive pipes should all be escaped", SecurityUtils.escapePipe("|||"), equalTo("\\|\\|\\|"));
+    }
+
 }