Skip to content

Commit f722c1a

Browse files
github-actions[bot]github-actions[bot]
committed
Escape/Unescape pipe UserInfo in ThreadContext (#804)
* Escape/Unescape pipe UserInfo in ThreadContext (#801) Signed-off-by: Shikhar Jain <[email protected]> * spotless Signed-off-by: Shikhar Jain <[email protected]> --------- Signed-off-by: Shikhar Jain <[email protected]> (cherry picked from commit 12d4357) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent ad42ac8 commit f722c1a

File tree

6 files changed

+251
-9
lines changed

6 files changed

+251
-9
lines changed

‎src/main/java/org/opensearch/commons/InjectSecurity.java

+8-4
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import static org.opensearch.commons.ConfigConstants.INJECTED_USER;
99
import static org.opensearch.commons.ConfigConstants.OPENSEARCH_SECURITY_INJECTED_ROLES;
1010
import static org.opensearch.commons.ConfigConstants.OPENSEARCH_SECURITY_USE_INJECTED_USER_FOR_PLUGINS;
11+
import static org.opensearch.commons.authuser.Utils.escapePipe;
1112

1213
import java.util.List;
1314
import java.util.StringJoiner;
@@ -143,13 +144,16 @@ public void injectUserInfo(final User user) {
143144
);
144145
return;
145146
}
147+
146148
StringJoiner joiner = new StringJoiner("|");
147-
joiner.add(user.getName());
148-
joiner.add(java.lang.String.join(",", user.getBackendRoles()));
149-
joiner.add(java.lang.String.join(",", user.getRoles()));
149+
// Escape pipe characters in all fields
150+
joiner.add(escapePipe(user.getName()));
151+
joiner.add(escapePipe(java.lang.String.join(",", user.getBackendRoles())));
152+
joiner.add(escapePipe(java.lang.String.join(",", user.getRoles())));
153+
150154
String requestedTenant = user.getRequestedTenant();
151155
if (!Strings.isNullOrEmpty(requestedTenant)) {
152-
joiner.add(requestedTenant);
156+
joiner.add(escapePipe(requestedTenant));
153157
}
154158
threadContext.putTransient(ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT, joiner.toString());
155159
}

‎src/main/java/org/opensearch/commons/authuser/User.java

+9-5
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package org.opensearch.commons.authuser;
77

8+
import static org.opensearch.commons.authuser.Utils.unescapePipe;
89
import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken;
910

1011
import java.io.IOException;
@@ -14,6 +15,7 @@
1415
import java.util.List;
1516
import java.util.Map;
1617
import java.util.Objects;
18+
import java.util.stream.Collectors;
1719

1820
import org.apache.http.util.EntityUtils;
1921
import org.opensearch.client.Response;
@@ -165,24 +167,26 @@ public static User parse(final String userString) {
165167
return null;
166168
}
167169

168-
String[] strs = userString.split("\\|");
170+
// Split on unescaped pipes (negative lookbehind for backslash)
171+
String[] strs = userString.split("(?<!\\\\)\\|");
169172
if ((strs.length == 0) || (Strings.isNullOrEmpty(strs[0]))) {
170173
return null;
171174
}
172175

173-
String userName = strs[0].trim();
176+
// Unescape the values
177+
String userName = unescapePipe(strs[0].trim());
174178
List<String> backendRoles = new ArrayList<>();
175179
List<String> roles = new ArrayList<>();
176180
String requestedTenant = null;
177181

178182
if ((strs.length > 1) && !Strings.isNullOrEmpty(strs[1])) {
179-
backendRoles.addAll(Arrays.asList(strs[1].split(",")));
183+
backendRoles.addAll(Arrays.stream(strs[1].split(",")).map(Utils::unescapePipe).collect(Collectors.toList()));
180184
}
181185
if ((strs.length > 2) && !Strings.isNullOrEmpty(strs[2])) {
182-
roles.addAll(Arrays.asList(strs[2].split(",")));
186+
roles.addAll(Arrays.stream(strs[2].split(",")).map(Utils::unescapePipe).collect(Collectors.toList()));
183187
}
184188
if ((strs.length > 3) && !Strings.isNullOrEmpty(strs[3])) {
185-
requestedTenant = strs[3].trim();
189+
requestedTenant = unescapePipe(strs[3].trim());
186190
}
187191
return new User(userName, backendRoles, roles, Arrays.asList(), requestedTenant);
188192
}

‎src/main/java/org/opensearch/commons/authuser/Utils.java

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.commons.authuser;
7+
8+
public final class Utils {
9+
10+
// Helper method to escape pipe characters
11+
public static String escapePipe(String input) {
12+
if (input == null) {
13+
return "";
14+
}
15+
return input.replace("|", "\\|");
16+
}
17+
18+
// Helper method to un-escape pipe characters
19+
public static String unescapePipe(String input) {
20+
if (input == null) {
21+
return "";
22+
}
23+
return input.replace("\\|", "|");
24+
}
25+
}

‎src/test/java/org/opensearch/commons/InjectSecurityTest.java

+36
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,42 @@ public void testInjectUserInfo() {
124124
assertNull(threadContext.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT));
125125
}
126126

127+
@Test
128+
public void testInjectUserInfoWithPipeInUserName() {
129+
Settings settings = Settings.builder().build();
130+
Settings headerSettings = Settings.builder().put("request.headers.default", "1").build();
131+
ThreadContext threadContext = new ThreadContext(headerSettings);
132+
threadContext.putHeader("name", "opendistro");
133+
threadContext.putTransient("ctx.name", "plugin");
134+
135+
assertEquals("1", threadContext.getHeader("default"));
136+
assertEquals("opendistro", threadContext.getHeader("name"));
137+
assertEquals("plugin", threadContext.getTransient("ctx.name"));
138+
139+
User user = new User(
140+
"Bob|test-pipe",
141+
List.of("backendRole1", "backendRole2"),
142+
List.of("role1", "role2"),
143+
List.of("attr1", "attr2"),
144+
"tenant1"
145+
);
146+
try (InjectSecurity helper = new InjectSecurity("test-name", null, threadContext)) {
147+
helper.injectUserInfo(user);
148+
assertEquals("1", threadContext.getHeader("default"));
149+
assertEquals("opendistro", threadContext.getHeader("name"));
150+
assertEquals("plugin", threadContext.getTransient("ctx.name"));
151+
assertNotNull(threadContext.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT));
152+
assertEquals(
153+
"Bob\\|test-pipe|backendRole1,backendRole2|role1,role2|tenant1",
154+
threadContext.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT)
155+
);
156+
}
157+
assertEquals("1", threadContext.getHeader("default"));
158+
assertEquals("opendistro", threadContext.getHeader("name"));
159+
assertEquals("plugin", threadContext.getTransient("ctx.name"));
160+
assertNull(threadContext.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT));
161+
}
162+
127163
@Test
128164
public void testInjectProperty() {
129165
Settings settings = Settings.builder().put(OPENSEARCH_SECURITY_USE_INJECTED_USER_FOR_PLUGINS, false).build();

‎src/test/java/org/opensearch/commons/authuser/UserTest.java

+81
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,87 @@ public void testParseUserStringMalformed() {
205205
assertNull(user);
206206
}
207207

208+
@Test
209+
public void testParseUserStringWithPipeInUserName() {
210+
ThreadContext tc = new ThreadContext(Settings.EMPTY);
211+
tc.putTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT, "myuser\\|test-pipe|bckrole1,bckrol2|role1,role2|myTenant");
212+
String str = tc.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT);
213+
User user = User.parse(str);
214+
215+
assertEquals("myuser|test-pipe", user.getName());
216+
assertEquals(2, user.getBackendRoles().size());
217+
assertEquals(2, user.getRoles().size());
218+
assertTrue(user.getRoles().contains("role1"));
219+
assertTrue(user.getRoles().contains("role2"));
220+
assertEquals("myTenant", user.getRequestedTenant());
221+
}
222+
223+
@Test
224+
public void testParseUserStringWithMultiplePipesInUserName() {
225+
ThreadContext tc = new ThreadContext(Settings.EMPTY);
226+
tc
227+
.putTransient(
228+
OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT,
229+
"myuser\\|test-pipe\\|test-pipe2|bckrole1,bckrol2|role1,role2|myTenant"
230+
);
231+
String str = tc.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT);
232+
User user = User.parse(str);
233+
234+
assertEquals("myuser|test-pipe|test-pipe2", user.getName());
235+
assertEquals(2, user.getBackendRoles().size());
236+
assertEquals(2, user.getRoles().size());
237+
assertTrue(user.getRoles().contains("role1"));
238+
assertTrue(user.getRoles().contains("role2"));
239+
assertEquals("myTenant", user.getRequestedTenant());
240+
}
241+
242+
@Test
243+
public void testParseUserStringWithPipeInBackedRoleName() {
244+
ThreadContext tc = new ThreadContext(Settings.EMPTY);
245+
tc.putTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT, "myuser|bckrole1\\|br1,bckrole2\\|br2|role1,role2|myTenant");
246+
String str = tc.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT);
247+
User user = User.parse(str);
248+
249+
assertEquals("myuser", user.getName());
250+
assertEquals(2, user.getBackendRoles().size());
251+
assertTrue(user.getBackendRoles().contains("bckrole1|br1"));
252+
assertTrue(user.getBackendRoles().contains("bckrole2|br2"));
253+
assertEquals(2, user.getRoles().size());
254+
assertTrue(user.getRoles().contains("role1"));
255+
assertTrue(user.getRoles().contains("role2"));
256+
assertEquals("myTenant", user.getRequestedTenant());
257+
}
258+
259+
@Test
260+
public void testParseUserStringWithPipeInRoleName() {
261+
ThreadContext tc = new ThreadContext(Settings.EMPTY);
262+
tc.putTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT, "myuser|bckrole1,bckrol2|role1\\|r1,role2\\|r2|myTenant");
263+
String str = tc.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT);
264+
User user = User.parse(str);
265+
266+
assertEquals("myuser", user.getName());
267+
assertEquals(2, user.getBackendRoles().size());
268+
assertEquals(2, user.getRoles().size());
269+
assertTrue(user.getRoles().contains("role1|r1"));
270+
assertTrue(user.getRoles().contains("role2|r2"));
271+
assertEquals("myTenant", user.getRequestedTenant());
272+
}
273+
274+
@Test
275+
public void testParseUserStringWithPipeInTenantName() {
276+
ThreadContext tc = new ThreadContext(Settings.EMPTY);
277+
tc.putTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT, "myuser|bckrole1,bckrol2|role1,role2|myTenant\\|t1");
278+
String str = tc.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT);
279+
User user = User.parse(str);
280+
281+
assertEquals("myuser", user.getName());
282+
assertEquals(2, user.getBackendRoles().size());
283+
assertEquals(2, user.getRoles().size());
284+
assertTrue(user.getRoles().contains("role1"));
285+
assertTrue(user.getRoles().contains("role2"));
286+
assertEquals("myTenant|t1", user.getRequestedTenant());
287+
}
288+
208289
@Test
209290
public void testUserIsAdminDnTrue() {
210291
Settings settings = Settings

‎src/test/java/org/opensearch/commons/authuser/UtilsTest.java

+92
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.commons.authuser;
7+
8+
import static org.junit.jupiter.api.Assertions.assertEquals;
9+
10+
import org.junit.jupiter.api.Assertions;
11+
import org.junit.jupiter.api.Test;
12+
13+
public class UtilsTest {
14+
15+
@Test
16+
public void testEscapePipe_NullInput() {
17+
assertEquals("", Utils.escapePipe(null));
18+
}
19+
20+
@Test
21+
public void testEscapePipe_EmptyString() {
22+
assertEquals("", Utils.escapePipe(""));
23+
}
24+
25+
@Test
26+
public void testEscapePipe_NoPipes() {
27+
assertEquals("normal text", Utils.escapePipe("normal text"));
28+
}
29+
30+
@Test
31+
public void testEscapePipe_SinglePipe() {
32+
assertEquals("before\\|after", Utils.escapePipe("before|after"));
33+
}
34+
35+
@Test
36+
public void testEscapePipe_MultiplePipes() {
37+
assertEquals("a\\|b\\|c", Utils.escapePipe("a|b|c"));
38+
}
39+
40+
@Test
41+
public void testEscapePipe_PipeAtStart() {
42+
assertEquals("\\|text", Utils.escapePipe("|text"));
43+
}
44+
45+
@Test
46+
public void testEscapePipe_PipeAtEnd() {
47+
assertEquals("text\\|", Utils.escapePipe("text|"));
48+
}
49+
50+
@Test
51+
public void testUnescapePipe_NullInput() {
52+
assertEquals("", Utils.unescapePipe(null));
53+
}
54+
55+
@Test
56+
public void testUnescapePipe_EmptyString() {
57+
assertEquals("", Utils.unescapePipe(""));
58+
}
59+
60+
@Test
61+
public void testUnescapePipe_NoEscapedPipes() {
62+
Assertions.assertEquals("normal text", Utils.unescapePipe("normal text"));
63+
}
64+
65+
@Test
66+
public void testUnescapePipe_SingleEscapedPipe() {
67+
assertEquals("before|after", Utils.unescapePipe("before\\|after"));
68+
}
69+
70+
@Test
71+
public void testUnescapePipe_MultipleEscapedPipes() {
72+
assertEquals("a|b|c", Utils.unescapePipe("a\\|b\\|c"));
73+
}
74+
75+
@Test
76+
public void testUnescapePipe_EscapedPipeAtStart() {
77+
assertEquals("|text", Utils.unescapePipe("\\|text"));
78+
}
79+
80+
@Test
81+
public void testUnescapePipe_EscapedPipeAtEnd() {
82+
assertEquals("text|", Utils.unescapePipe("text\\|"));
83+
}
84+
85+
@Test
86+
public void testRoundTrip_ComplexString() {
87+
String original = "user|with|pipes";
88+
String escaped = Utils.escapePipe(original);
89+
String unescaped = Utils.unescapePipe(escaped);
90+
assertEquals(original, unescaped);
91+
}
92+
}

0 commit comments

Comments
 (0)