Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.x] Escape/Unescape pipe UserInfo in ThreadContext #802

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions src/main/java/org/opensearch/commons/InjectSecurity.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand Down
13 changes: 8 additions & 5 deletions src/main/java/org/opensearch/commons/authuser/User.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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("(?<!\\\\)\\|");
if ((strs.length == 0) || (Strings.isNullOrEmpty(strs[0]))) {
return null;
}

String userName = strs[0].trim();
// Unescape the values
String userName = unescapePipe(strs[0].trim());
List<String> backendRoles = new ArrayList<>();
List<String> 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);
}
Expand Down
25 changes: 25 additions & 0 deletions src/main/java/org/opensearch/commons/authuser/Utils.java
Original file line number Diff line number Diff line change
@@ -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("\\|", "|");
}
}
36 changes: 36 additions & 0 deletions src/test/java/org/opensearch/commons/InjectSecurityTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
81 changes: 81 additions & 0 deletions src/test/java/org/opensearch/commons/authuser/UserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
92 changes: 92 additions & 0 deletions src/test/java/org/opensearch/commons/authuser/UtilsTest.java
Original file line number Diff line number Diff line change
@@ -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);
}
}
Loading