diff --git a/src/main/java/org/opensearch/commons/InjectSecurity.java b/src/main/java/org/opensearch/commons/InjectSecurity.java index e6b283d9..b4a354ad 100644 --- a/src/main/java/org/opensearch/commons/InjectSecurity.java +++ b/src/main/java/org/opensearch/commons/InjectSecurity.java @@ -8,6 +8,7 @@ import static org.opensearch.commons.ConfigConstants.INJECTED_USER; import static org.opensearch.commons.ConfigConstants.OPENSEARCH_SECURITY_INJECTED_ROLES; import static org.opensearch.commons.ConfigConstants.OPENSEARCH_SECURITY_USE_INJECTED_USER_FOR_PLUGINS; +import static org.opensearch.commons.authuser.Utils.escapePipe; import java.util.List; import java.util.StringJoiner; @@ -143,13 +144,16 @@ public void injectUserInfo(final User user) { ); return; } + StringJoiner joiner = new StringJoiner("|"); - joiner.add(user.getName()); - joiner.add(java.lang.String.join(",", user.getBackendRoles())); - joiner.add(java.lang.String.join(",", user.getRoles())); + // Escape pipe characters in all fields + joiner.add(escapePipe(user.getName())); + joiner.add(escapePipe(java.lang.String.join(",", user.getBackendRoles()))); + joiner.add(escapePipe(java.lang.String.join(",", user.getRoles()))); + String requestedTenant = user.getRequestedTenant(); if (!Strings.isNullOrEmpty(requestedTenant)) { - joiner.add(requestedTenant); + joiner.add(escapePipe(requestedTenant)); } threadContext.putTransient(ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT, joiner.toString()); } diff --git a/src/main/java/org/opensearch/commons/authuser/User.java b/src/main/java/org/opensearch/commons/authuser/User.java index a203b33d..99d559ce 100644 --- a/src/main/java/org/opensearch/commons/authuser/User.java +++ b/src/main/java/org/opensearch/commons/authuser/User.java @@ -5,6 +5,7 @@ package org.opensearch.commons.authuser; +import static org.opensearch.commons.authuser.Utils.unescapePipe; import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; import java.io.IOException; @@ -165,24 +166,26 @@ public static User parse(final String userString) { return null; } - String[] strs = userString.split("\\|"); + // Split on unescaped pipes (negative lookbehind for backslash) + String[] strs = userString.split("(? backendRoles = new ArrayList<>(); List roles = new ArrayList<>(); String requestedTenant = null; if ((strs.length > 1) && !Strings.isNullOrEmpty(strs[1])) { - backendRoles.addAll(Arrays.asList(strs[1].split(","))); + backendRoles.addAll(Arrays.stream(strs[1].split(",")).map(Utils::unescapePipe).toList()); } if ((strs.length > 2) && !Strings.isNullOrEmpty(strs[2])) { - roles.addAll(Arrays.asList(strs[2].split(","))); + roles.addAll(Arrays.stream(strs[2].split(",")).map(Utils::unescapePipe).toList()); } if ((strs.length > 3) && !Strings.isNullOrEmpty(strs[3])) { - requestedTenant = strs[3].trim(); + requestedTenant = unescapePipe(strs[3].trim()); } return new User(userName, backendRoles, roles, Arrays.asList(), requestedTenant); } diff --git a/src/main/java/org/opensearch/commons/authuser/Utils.java b/src/main/java/org/opensearch/commons/authuser/Utils.java new file mode 100644 index 00000000..ad04390e --- /dev/null +++ b/src/main/java/org/opensearch/commons/authuser/Utils.java @@ -0,0 +1,25 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.commons.authuser; + +public final class Utils { + + // Helper method to escape pipe characters + public static String escapePipe(String input) { + if (input == null) { + return ""; + } + return input.replace("|", "\\|"); + } + + // Helper method to un-escape pipe characters + public static String unescapePipe(String input) { + if (input == null) { + return ""; + } + return input.replace("\\|", "|"); + } +} diff --git a/src/test/java/org/opensearch/commons/InjectSecurityTest.java b/src/test/java/org/opensearch/commons/InjectSecurityTest.java index b2dea7f3..19cc365c 100644 --- a/src/test/java/org/opensearch/commons/InjectSecurityTest.java +++ b/src/test/java/org/opensearch/commons/InjectSecurityTest.java @@ -124,6 +124,42 @@ public void testInjectUserInfo() { assertNull(threadContext.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT)); } + @Test + public void testInjectUserInfoWithPipeInUserName() { + Settings settings = Settings.builder().build(); + Settings headerSettings = Settings.builder().put("request.headers.default", "1").build(); + ThreadContext threadContext = new ThreadContext(headerSettings); + threadContext.putHeader("name", "opendistro"); + threadContext.putTransient("ctx.name", "plugin"); + + assertEquals("1", threadContext.getHeader("default")); + assertEquals("opendistro", threadContext.getHeader("name")); + assertEquals("plugin", threadContext.getTransient("ctx.name")); + + User user = new User( + "Bob|test-pipe", + List.of("backendRole1", "backendRole2"), + List.of("role1", "role2"), + List.of("attr1", "attr2"), + "tenant1" + ); + try (InjectSecurity helper = new InjectSecurity("test-name", null, threadContext)) { + helper.injectUserInfo(user); + assertEquals("1", threadContext.getHeader("default")); + assertEquals("opendistro", threadContext.getHeader("name")); + assertEquals("plugin", threadContext.getTransient("ctx.name")); + assertNotNull(threadContext.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT)); + assertEquals( + "Bob\\|test-pipe|backendRole1,backendRole2|role1,role2|tenant1", + threadContext.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT) + ); + } + assertEquals("1", threadContext.getHeader("default")); + assertEquals("opendistro", threadContext.getHeader("name")); + assertEquals("plugin", threadContext.getTransient("ctx.name")); + assertNull(threadContext.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT)); + } + @Test public void testInjectProperty() { Settings settings = Settings.builder().put(OPENSEARCH_SECURITY_USE_INJECTED_USER_FOR_PLUGINS, false).build(); diff --git a/src/test/java/org/opensearch/commons/authuser/UserTest.java b/src/test/java/org/opensearch/commons/authuser/UserTest.java index df4e6602..1e0db06c 100644 --- a/src/test/java/org/opensearch/commons/authuser/UserTest.java +++ b/src/test/java/org/opensearch/commons/authuser/UserTest.java @@ -205,6 +205,87 @@ public void testParseUserStringMalformed() { assertNull(user); } + @Test + public void testParseUserStringWithPipeInUserName() { + ThreadContext tc = new ThreadContext(Settings.EMPTY); + tc.putTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT, "myuser\\|test-pipe|bckrole1,bckrol2|role1,role2|myTenant"); + String str = tc.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT); + User user = User.parse(str); + + assertEquals("myuser|test-pipe", user.getName()); + assertEquals(2, user.getBackendRoles().size()); + assertEquals(2, user.getRoles().size()); + assertTrue(user.getRoles().contains("role1")); + assertTrue(user.getRoles().contains("role2")); + assertEquals("myTenant", user.getRequestedTenant()); + } + + @Test + public void testParseUserStringWithMultiplePipesInUserName() { + ThreadContext tc = new ThreadContext(Settings.EMPTY); + tc + .putTransient( + OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT, + "myuser\\|test-pipe\\|test-pipe2|bckrole1,bckrol2|role1,role2|myTenant" + ); + String str = tc.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT); + User user = User.parse(str); + + assertEquals("myuser|test-pipe|test-pipe2", user.getName()); + assertEquals(2, user.getBackendRoles().size()); + assertEquals(2, user.getRoles().size()); + assertTrue(user.getRoles().contains("role1")); + assertTrue(user.getRoles().contains("role2")); + assertEquals("myTenant", user.getRequestedTenant()); + } + + @Test + public void testParseUserStringWithPipeInBackedRoleName() { + ThreadContext tc = new ThreadContext(Settings.EMPTY); + tc.putTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT, "myuser|bckrole1\\|br1,bckrole2\\|br2|role1,role2|myTenant"); + String str = tc.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT); + User user = User.parse(str); + + assertEquals("myuser", user.getName()); + assertEquals(2, user.getBackendRoles().size()); + assertTrue(user.getBackendRoles().contains("bckrole1|br1")); + assertTrue(user.getBackendRoles().contains("bckrole2|br2")); + assertEquals(2, user.getRoles().size()); + assertTrue(user.getRoles().contains("role1")); + assertTrue(user.getRoles().contains("role2")); + assertEquals("myTenant", user.getRequestedTenant()); + } + + @Test + public void testParseUserStringWithPipeInRoleName() { + ThreadContext tc = new ThreadContext(Settings.EMPTY); + tc.putTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT, "myuser|bckrole1,bckrol2|role1\\|r1,role2\\|r2|myTenant"); + String str = tc.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT); + User user = User.parse(str); + + assertEquals("myuser", user.getName()); + assertEquals(2, user.getBackendRoles().size()); + assertEquals(2, user.getRoles().size()); + assertTrue(user.getRoles().contains("role1|r1")); + assertTrue(user.getRoles().contains("role2|r2")); + assertEquals("myTenant", user.getRequestedTenant()); + } + + @Test + public void testParseUserStringWithPipeInTenantName() { + ThreadContext tc = new ThreadContext(Settings.EMPTY); + tc.putTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT, "myuser|bckrole1,bckrol2|role1,role2|myTenant\\|t1"); + String str = tc.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT); + User user = User.parse(str); + + assertEquals("myuser", user.getName()); + assertEquals(2, user.getBackendRoles().size()); + assertEquals(2, user.getRoles().size()); + assertTrue(user.getRoles().contains("role1")); + assertTrue(user.getRoles().contains("role2")); + assertEquals("myTenant|t1", user.getRequestedTenant()); + } + @Test public void testUserIsAdminDnTrue() { Settings settings = Settings diff --git a/src/test/java/org/opensearch/commons/authuser/UtilsTest.java b/src/test/java/org/opensearch/commons/authuser/UtilsTest.java new file mode 100644 index 00000000..7c1ff2ed --- /dev/null +++ b/src/test/java/org/opensearch/commons/authuser/UtilsTest.java @@ -0,0 +1,92 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.commons.authuser; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class UtilsTest { + + @Test + public void testEscapePipe_NullInput() { + assertEquals("", Utils.escapePipe(null)); + } + + @Test + public void testEscapePipe_EmptyString() { + assertEquals("", Utils.escapePipe("")); + } + + @Test + public void testEscapePipe_NoPipes() { + assertEquals("normal text", Utils.escapePipe("normal text")); + } + + @Test + public void testEscapePipe_SinglePipe() { + assertEquals("before\\|after", Utils.escapePipe("before|after")); + } + + @Test + public void testEscapePipe_MultiplePipes() { + assertEquals("a\\|b\\|c", Utils.escapePipe("a|b|c")); + } + + @Test + public void testEscapePipe_PipeAtStart() { + assertEquals("\\|text", Utils.escapePipe("|text")); + } + + @Test + public void testEscapePipe_PipeAtEnd() { + assertEquals("text\\|", Utils.escapePipe("text|")); + } + + @Test + public void testUnescapePipe_NullInput() { + assertEquals("", Utils.unescapePipe(null)); + } + + @Test + public void testUnescapePipe_EmptyString() { + assertEquals("", Utils.unescapePipe("")); + } + + @Test + public void testUnescapePipe_NoEscapedPipes() { + Assertions.assertEquals("normal text", Utils.unescapePipe("normal text")); + } + + @Test + public void testUnescapePipe_SingleEscapedPipe() { + assertEquals("before|after", Utils.unescapePipe("before\\|after")); + } + + @Test + public void testUnescapePipe_MultipleEscapedPipes() { + assertEquals("a|b|c", Utils.unescapePipe("a\\|b\\|c")); + } + + @Test + public void testUnescapePipe_EscapedPipeAtStart() { + assertEquals("|text", Utils.unescapePipe("\\|text")); + } + + @Test + public void testUnescapePipe_EscapedPipeAtEnd() { + assertEquals("text|", Utils.unescapePipe("text\\|")); + } + + @Test + public void testRoundTrip_ComplexString() { + String original = "user|with|pipes"; + String escaped = Utils.escapePipe(original); + String unescaped = Utils.unescapePipe(escaped); + assertEquals(original, unescaped); + } +}