Directly store permissions in the cache

Signed-off-by: Derek Ho <[email protected]>
derek-ho · Dec 30, 2024 · 665b9e9 · 665b9e9
1 parent 22cfbe8
commit 665b9e9
Show file tree

Hide file tree

Showing 8 changed files with 106 additions and 132 deletions.
diff --git a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexListenerCache.java b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexListenerCache.java
@@ -9,8 +9,7 @@
 package org.opensearch.security.action.apitokens;
 
 import java.io.IOException;
-import java.util.HashSet;
-import java.util.Set;
+import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
 import org.apache.logging.log4j.LogManager;
@@ -36,7 +35,7 @@ public class ApiTokenIndexListenerCache implements IndexingOperationListener {
     private static final ApiTokenIndexListenerCache INSTANCE = new ApiTokenIndexListenerCache();
     private final ConcurrentHashMap<String, String> idToJtiMap = new ConcurrentHashMap<>();
 
-    private Set<String> jtis = new HashSet<String>();
+    private Map<String, Permissions> jtis = new ConcurrentHashMap<>();
 
     private boolean initialized;
 
@@ -81,7 +80,7 @@ public void postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult re
                 .createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, sourceRef.streamInput());
 
             ApiToken token = ApiToken.fromXContent(parser);
-            jtis.add(token.getJti());
+            jtis.put(token.getJti(), new Permissions(token.getClusterPermissions(), token.getIndexPermissions()));
             idToJtiMap.put(index.id(), token.getJti());
 
         } catch (IOException e) {
@@ -106,7 +105,8 @@ public void postDelete(ShardId shardId, Engine.Delete delete, Engine.DeleteResul
         }
     }
 
-    public Set<String> getJtis() {
+    public Map<String, Permissions> getJtis() {
         return jtis;
     }
+
 }
diff --git a/src/main/java/org/opensearch/security/action/apitokens/Permissions.java b/src/main/java/org/opensearch/security/action/apitokens/Permissions.java
@@ -0,0 +1,40 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.security.action.apitokens;
+
+import java.util.List;
+
+public class Permissions {
+    private List<String> clusterPerm;
+    private List<ApiToken.IndexPermission> indexPermission;
+
+    // Constructor
+    public Permissions(List<String> clusterPerm, List<ApiToken.IndexPermission> indexPermission) {
+        this.clusterPerm = clusterPerm;
+        this.indexPermission = indexPermission;
+    }
+
+    // Getters and setters
+    public List<String> getClusterPerm() {
+        return clusterPerm;
+    }
+
+    public void setClusterPerm(List<String> clusterPerm) {
+        this.clusterPerm = clusterPerm;
+    }
+
+    public List<ApiToken.IndexPermission> getIndexPermission() {
+        return indexPermission;
+    }
+
+    public void setIndexPermission(List<ApiToken.IndexPermission> indexPermission) {
+        this.indexPermission = indexPermission;
+    }
+
+}
diff --git a/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java b/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java
@@ -76,8 +76,6 @@
 public class SecurityRestFilter {
 
     protected final Logger log = LogManager.getLogger(this.getClass());
-    public static final String API_TOKEN_CLUSTERPERM_KEY = "security.api_token.clusterperm";
-    public static final String API_TOKEN_INDEXPERM_KEY = "security.api_token.indexactions";
     private final BackendRegistry registry;
     private final RestLayerPrivilegesEvaluator evaluator;
     private final AuditLog auditLog;
@@ -234,7 +232,6 @@ void authorizeRequest(RestHandler original, SecurityRequestChannel request, User
                 .addAll(route.actionNames() != null ? route.actionNames() : Collections.emptySet())
                 .add(route.name())
                 .build();
-
             pres = evaluator.evaluate(user, route.name(), actionNames);
 
             if (log.isDebugEnabled()) {

diff --git a/src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java b/src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java
@@ -11,10 +11,8 @@
 
 package org.opensearch.security.http;
 
-import java.io.IOException;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 import java.util.regex.Matcher;
@@ -29,11 +27,6 @@
 import org.opensearch.SpecialPermission;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.util.concurrent.ThreadContext;
-import org.opensearch.common.xcontent.XContentType;
-import org.opensearch.core.xcontent.DeprecationHandler;
-import org.opensearch.core.xcontent.NamedXContentRegistry;
-import org.opensearch.core.xcontent.XContentParser;
-import org.opensearch.security.action.apitokens.ApiToken;
 import org.opensearch.security.action.apitokens.ApiTokenIndexListenerCache;
 import org.opensearch.security.auth.HTTPAuthenticator;
 import org.opensearch.security.authtoken.jwt.EncryptionDecryptionUtil;
@@ -50,8 +43,6 @@
 
 import static org.opensearch.security.OpenSearchSecurityPlugin.LEGACY_OPENDISTRO_PREFIX;
 import static org.opensearch.security.OpenSearchSecurityPlugin.PLUGINS_PREFIX;
-import static org.opensearch.security.filter.SecurityRestFilter.API_TOKEN_CLUSTERPERM_KEY;
-import static org.opensearch.security.filter.SecurityRestFilter.API_TOKEN_INDEXPERM_KEY;
 import static org.opensearch.security.util.AuthTokenUtils.isAccessToRestrictedEndpoints;
 
 public class ApiTokenAuthenticator implements HTTPAuthenticator {
@@ -113,49 +104,6 @@ private JwtParserBuilder initParserBuilder(final String signingKey) {
         return jwtParserBuilder;
     }
 
-    private String extractClusterPermissionsFromClaims(Claims claims) {
-        Object cp = claims.get("cp");
-        String clusterPermissions = "";
-
-        if (cp != null) {
-            clusterPermissions = encryptionUtil.decrypt(cp.toString());
-        } else {
-            log.warn("This is a malformed Api Token");
-        }
-
-        return clusterPermissions;
-    }
-
-    private List<ApiToken.IndexPermission> extractIndexPermissionFromClaims(Claims claims) throws IOException {
-        Object ip = claims.get("ip");
-
-        if (ip != null) {
-            String decryptedPermissions = encryptionUtil.decrypt(ip.toString());
-
-            try (
-                XContentParser parser = XContentType.JSON.xContent()
-                    .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, decryptedPermissions)
-            ) {
-
-                // Use built-in array parsing
-                List<ApiToken.IndexPermission> permissions = new ArrayList<>();
-
-                // Move to start of array
-                parser.nextToken();  // START_ARRAY
-                while (parser.nextToken() != XContentParser.Token.END_ARRAY) {
-                    permissions.add(ApiToken.IndexPermission.fromXContent(parser));
-                }
-                return permissions;
-            } catch (Exception e) {
-                log.error("Error extracting index permissions", e);
-                return List.of();
-            }
-
-        }
-
-        return List.of();
-    }
-
     @Override
     @SuppressWarnings("removal")
     public AuthCredentials extractCredentials(final SecurityRequest request, final ThreadContext context)
@@ -193,8 +141,8 @@ private AuthCredentials extractCredentials0(final SecurityRequest request, final
         }
 
         // TODO: handle revocation different from deletion?
-        if (!cache.getJtis().contains(encryptionUtil.encrypt(jwtToken))) {
-            log.debug("Token is not allowlisted");
+        if (!cache.getJtis().containsKey(encryptionUtil.encrypt(jwtToken))) {
+            log.error("Token is not allowlisted");
             return null;
         }
 
@@ -213,23 +161,16 @@ private AuthCredentials extractCredentials0(final SecurityRequest request, final
                 return null;
             }
 
-            String clusterPermissions = extractClusterPermissionsFromClaims(claims);
-            List<ApiToken.IndexPermission> indexPermissions = extractIndexPermissionFromClaims(claims);
-
-            final AuthCredentials ac = new AuthCredentials(subject, List.of(), "").markComplete();
-
-            context.putTransient(API_TOKEN_CLUSTERPERM_KEY, clusterPermissions);
-            context.putTransient(API_TOKEN_INDEXPERM_KEY, indexPermissions);
+            final AuthCredentials ac = new AuthCredentials("apitoken_" + subject + ":" + encryptionUtil.encrypt(jwtToken), List.of(), "")
+                .markComplete();
 
             return ac;
 
         } catch (WeakKeyException e) {
             log.error("Cannot authenticate user with JWT because of ", e);
             return null;
         } catch (Exception e) {
-            if (log.isDebugEnabled()) {
-                log.debug("Invalid or expired JWT token.", e);
-            }
+            log.error("Invalid or expired JWT token.", e);
         }
 
         // Return null for the authentication failure

diff --git a/src/main/java/org/opensearch/security/privileges/ActionPrivileges.java b/src/main/java/org/opensearch/security/privileges/ActionPrivileges.java
@@ -165,17 +165,23 @@ public PrivilegesEvaluatorResponse hasIndexPrivilege(
 
         // API Token Authz
         // TODO: this is very naive implementation
-        if (context.getIndexPermissions() != null) {
-            List<ApiToken.IndexPermission> indexPermissions = context.getIndexPermissions();
-
-            boolean hasPermission = indexPermissions.stream().anyMatch(permission -> {
-                boolean hasAllActions = new HashSet<>(permission.getAllowedActions()).containsAll(actions);
-                boolean hasAllIndices = new HashSet<>(permission.getIndexPatterns()).containsAll(resolvedIndices.getAllIndices());
-                return hasAllActions && hasAllIndices;
-            });
-
-            if (hasPermission) {
-                return PrivilegesEvaluatorResponse.ok();
+        if (context.getUser().getName().startsWith("apitoken")) {
+            String jti = context.getUser().getName().split(":")[1];
+            if (context.getApiTokenIndexListenerCache().getJtis().get(jti) != null) {
+                List<ApiToken.IndexPermission> indexPermissions = context.getApiTokenIndexListenerCache()
+                    .getJtis()
+                    .get(jti)
+                    .getIndexPermission();
+
+                boolean hasPermission = indexPermissions.stream().anyMatch(permission -> {
+                    boolean hasAllActions = new HashSet<>(permission.getAllowedActions()).containsAll(actions);
+                    boolean hasAllIndices = new HashSet<>(permission.getIndexPatterns()).containsAll(resolvedIndices.getAllIndices());
+                    return hasAllActions && hasAllIndices;
+                });
+
+                if (hasPermission) {
+                    return PrivilegesEvaluatorResponse.ok();
+                }
             }
         }
 
@@ -426,8 +432,14 @@ PrivilegesEvaluatorResponse providesPrivilege(PrivilegesEvaluationContext contex
             }
 
             // 4: Evaluate api tokens
-            if (context.getClusterPermissions() != null && context.getClusterPermissions().contains(action)) {
-                return PrivilegesEvaluatorResponse.ok();
+            if (context.getUser().getName().startsWith("apitoken")) {
+                String jti = context.getUser().getName().split(":")[1];
+                log.info(context.getApiTokenIndexListenerCache().getJtis().get(jti).getClusterPerm().toString());
+
+                if (context.getApiTokenIndexListenerCache().getJtis().get(jti) != null
+                    && context.getApiTokenIndexListenerCache().getJtis().get(jti).getClusterPerm().contains(action)) {
+                    return PrivilegesEvaluatorResponse.ok();
+                }
             }
 
             return PrivilegesEvaluatorResponse.insufficient(action);
@@ -463,6 +475,14 @@ PrivilegesEvaluatorResponse providesExplicitPrivilege(PrivilegesEvaluationContex
                 }
             }
 
+            if (context.getUser().getName().startsWith("apitoken")) {
+                String jti = context.getUser().getName().split(":")[1];
+                if (context.getApiTokenIndexListenerCache().getJtis().get(jti) != null
+                    && context.getApiTokenIndexListenerCache().getJtis().get(jti).getClusterPerm().contains(action)) {
+                    return PrivilegesEvaluatorResponse.ok();
+                }
+            }
+
             return PrivilegesEvaluatorResponse.insufficient(action);
         }
 
@@ -499,6 +519,14 @@ PrivilegesEvaluatorResponse providesAnyPrivilege(PrivilegesEvaluationContext con
                 }
             }
 
+            if (context.getUser().getName().startsWith("apitoken")) {
+                String jti = context.getUser().getName().split(":")[1];
+                if (context.getApiTokenIndexListenerCache().getJtis().get(jti) != null
+                    && context.getApiTokenIndexListenerCache().getJtis().get(jti).getClusterPerm().stream().anyMatch(actions::contains)) {
+                    return PrivilegesEvaluatorResponse.ok();
+                }
+            }
+
             if (actions.size() == 1) {
                 return PrivilegesEvaluatorResponse.insufficient(actions.iterator().next());
             } else {

diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluationContext.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluationContext.java
@@ -11,7 +11,6 @@
 package org.opensearch.security.privileges;
 
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.function.Supplier;
 
@@ -21,7 +20,7 @@
 import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.metadata.IndexAbstraction;
 import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
-import org.opensearch.security.action.apitokens.ApiToken;
+import org.opensearch.security.action.apitokens.ApiTokenIndexListenerCache;
 import org.opensearch.security.resolver.IndexResolverReplacer;
 import org.opensearch.security.support.WildcardMatcher;
 import org.opensearch.security.user.User;
@@ -47,9 +46,7 @@ public class PrivilegesEvaluationContext {
     private final IndexResolverReplacer indexResolverReplacer;
     private final IndexNameExpressionResolver indexNameExpressionResolver;
     private final Supplier<ClusterState> clusterStateSupplier;
-    private List<String> clusterPermissions;
-    private List<ApiToken.IndexPermission> indexPermissions;
-
+    private final ApiTokenIndexListenerCache apiTokenIndexListenerCache = ApiTokenIndexListenerCache.getInstance();
     /**
      * This caches the ready to use WildcardMatcher instances for the current request. Many index patterns have
      * to be executed several times per request (for example first for action privileges, later for DLS). Thus,
@@ -177,19 +174,7 @@ public String toString() {
             + '}';
     }
 
-    public void setClusterPermissions(List<String> clusterPermissions) {
-        this.clusterPermissions = clusterPermissions;
-    }
-
-    public List<String> getClusterPermissions() {
-        return clusterPermissions;
-    }
-
-    public List<ApiToken.IndexPermission> getIndexPermissions() {
-        return indexPermissions;
-    }
-
-    public void setIndexPermissions(List<ApiToken.IndexPermission> indexPermissions) {
-        this.indexPermissions = indexPermissions;
+    public ApiTokenIndexListenerCache getApiTokenIndexListenerCache() {
+        return apiTokenIndexListenerCache;
     }
 }
diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java
@@ -86,7 +86,6 @@
 import org.opensearch.core.xcontent.NamedXContentRegistry;
 import org.opensearch.index.reindex.ReindexAction;
 import org.opensearch.script.mustache.RenderSearchTemplateAction;
-import org.opensearch.security.action.apitokens.ApiToken;
 import org.opensearch.security.auditlog.AuditLog;
 import org.opensearch.security.configuration.ClusterInfoHolder;
 import org.opensearch.security.configuration.ConfigurationRepository;
@@ -110,8 +109,6 @@
 import org.greenrobot.eventbus.Subscribe;
 
 import static org.opensearch.security.OpenSearchSecurityPlugin.traceAction;
-import static org.opensearch.security.filter.SecurityRestFilter.API_TOKEN_CLUSTERPERM_KEY;
-import static org.opensearch.security.filter.SecurityRestFilter.API_TOKEN_INDEXPERM_KEY;
 import static org.opensearch.security.support.ConfigConstants.OPENDISTRO_SECURITY_USER_INFO_THREAD_CONTEXT;
 
 public class PrivilegesEvaluator {
@@ -345,20 +342,6 @@ public PrivilegesEvaluatorResponse evaluate(PrivilegesEvaluationContext context)
             context.setMappedRoles(mappedRoles);
         }
 
-        // Extract cluster and index permissions from the api token thread context
-        // TODO: Add decryption here to make sure it is not injectable by anyone?
-        // TODO: This is only a naive implementation that does not support *
-        final String apiTokenClusterPermissions = threadContext.getTransient(API_TOKEN_CLUSTERPERM_KEY);
-        if (apiTokenClusterPermissions != null) {
-            List<String> clusterPermissions = Arrays.asList(apiTokenClusterPermissions.split(","));
-            context.setClusterPermissions(clusterPermissions);
-        }
-
-        final List<ApiToken.IndexPermission> apiTokenIndexPermissions = threadContext.getTransient(API_TOKEN_INDEXPERM_KEY);
-        if (apiTokenIndexPermissions != null) {
-            context.setIndexPermissions(apiTokenIndexPermissions);
-        }
-
         // Add the security roles for this user so that they can be used for DLS parameter substitution.
         user.addSecurityRoles(mappedRoles);
         setUserInfoInThreadContext(user);