[Backport 2.9] Reverse order of setUserInfoInThreadContext and addSec…

…urityRoles to resolve ConcurrentModificationException on bulk request (opensearch-project#3173) Backport cd699bb from opensearch-project#3094. Signed-off-by: Craig Perkins <[email protected]> Signed-off-by: Craig Perkins <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
peternied · Aug 14, 2023 · 6170fce · 6170fce
1 parent 07508a0
commit 6170fce
Showing 1 changed file with 3 additions and 4 deletions.
diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java
@@ -38,7 +38,6 @@
 import java.util.regex.Pattern;
 
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Sets;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.greenrobot.eventbus.Subscribe;
@@ -202,12 +201,12 @@ public boolean isInitialized() {
         return configModel != null && configModel.getSecurityRoles() != null && dcm != null;
     }
 
-    private void setUserInfoInThreadContext(User user, Set<String> mappedRoles) {
+    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(",", Sets.union(user.getSecurityRoles(), mappedRoles)));
+            joiner.add(String.join(",", user.getSecurityRoles()));
             String requestedTenant = user.getRequestedTenant();
             if (!Strings.isNullOrEmpty(requestedTenant)) {
                 joiner.add(requestedTenant);
@@ -260,9 +259,9 @@ public PrivilegesEvaluatorResponse evaluate(
         presponse.resolvedSecurityRoles.addAll(mappedRoles);
         final SecurityRoles securityRoles = getSecurityRoles(mappedRoles);
 
-        setUserInfoInThreadContext(user, mappedRoles);
         // Add the security roles for this user so that they can be used for DLS parameter substitution.
         user.addSecurityRoles(mappedRoles);
+        setUserInfoInThreadContext(user);
 
         final boolean isDebugEnabled = log.isDebugEnabled();
         if (isDebugEnabled) {