Fix

Signed-off-by: Derek Ho <[email protected]>
derek-ho · Jan 15, 2025 · 0927d99 · 0927d99
1 parent e44072c
commit 0927d99
Show file tree

Hide file tree

Showing 13 changed files with 38 additions and 16 deletions.
diff --git a/src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java b/src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java
@@ -53,6 +53,7 @@
 import org.mockito.Mockito;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.mockito.Mockito.mock;
 import static org.opensearch.security.privileges.PrivilegeEvaluatorResponseMatcher.isAllowed;
 import static org.opensearch.security.privileges.PrivilegeEvaluatorResponseMatcher.isForbidden;
 import static org.opensearch.security.privileges.PrivilegeEvaluatorResponseMatcher.isPartiallyOk;
@@ -1132,7 +1133,8 @@ static PrivilegesEvaluationContext ctxWithUserName(String userName, String... ro
             null,
             null,
             new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY)),
-            null
+            null,
+                mock(ApiTokenRepository.class)
         );
     }
 

diff --git a/src/integrationTest/java/org/opensearch/security/privileges/IndexPatternTest.java b/src/integrationTest/java/org/opensearch/security/privileges/IndexPatternTest.java
@@ -22,10 +22,12 @@
 import org.opensearch.cluster.metadata.Metadata;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.util.concurrent.ThreadContext;
+import org.opensearch.security.action.apitokens.ApiTokenRepository;
 import org.opensearch.security.resolver.IndexResolverReplacer;
 import org.opensearch.security.support.WildcardMatcher;
 import org.opensearch.security.user.User;
 
+import static org.mockito.Mockito.mock;
 import static org.opensearch.security.util.MockIndexMetadataBuilder.indices;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -246,7 +248,8 @@ private static PrivilegesEvaluationContext ctx() {
             null,
             indexResolverReplacer,
             indexNameExpressionResolver,
-            () -> CLUSTER_STATE
+            () -> CLUSTER_STATE,
+                mock(ApiTokenRepository.class)
         );
     }
 }
diff --git a/src/integrationTest/java/org/opensearch/security/privileges/RestEndpointPermissionTests.java b/src/integrationTest/java/org/opensearch/security/privileges/RestEndpointPermissionTests.java
@@ -44,6 +44,7 @@
 
 import org.opensearch.common.settings.Settings;
 import org.opensearch.security.DefaultObjectMapper;
+import org.opensearch.security.action.apitokens.ApiTokenRepository;
 import org.opensearch.security.dlic.rest.api.Endpoint;
 import org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.PermissionBuilder;
 import org.opensearch.security.securityconf.FlattenedActionGroups;
@@ -52,6 +53,7 @@
 import org.opensearch.security.securityconf.impl.v7.RoleV7;
 import org.opensearch.security.user.User;
 
+import static org.mockito.Mockito.mock;
 import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.CERTS_INFO_ACTION;
 import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.ENDPOINTS_WITH_PERMISSIONS;
 import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.RELOAD_CERTS_ACTION;
@@ -251,7 +253,7 @@ static SecurityDynamicConfiguration<RoleV7> createRolesConfig() throws IOExcepti
     }
 
     static PrivilegesEvaluationContext ctx(String... roles) {
-        return new PrivilegesEvaluationContext(new User("test_user"), ImmutableSet.copyOf(roles), null, null, null, null, null, null);
+        return new PrivilegesEvaluationContext(new User("test_user"), ImmutableSet.copyOf(roles), null, null, null, null, null, null, mock(ApiTokenRepository.class));
     }
 
 }
diff --git a/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/FieldMaskingTest.java b/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/FieldMaskingTest.java
@@ -22,6 +22,7 @@
 import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.metadata.Metadata;
 import org.opensearch.common.settings.Settings;
+import org.opensearch.security.action.apitokens.ApiTokenRepository;
 import org.opensearch.security.privileges.PrivilegesConfigurationValidationException;
 import org.opensearch.security.privileges.PrivilegesEvaluationContext;
 import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
@@ -30,6 +31,7 @@
 import org.opensearch.security.user.User;
 import org.opensearch.test.framework.TestSecurityConfig;
 
+import static org.mockito.Mockito.mock;
 import static org.opensearch.security.util.MockIndexMetadataBuilder.indices;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -123,7 +125,8 @@ static PrivilegesEvaluationContext ctx(String... roles) {
                 null,
                 null,
                 null,
-                () -> CLUSTER_STATE
+                () -> CLUSTER_STATE,
+                    mock(ApiTokenRepository.class)
             );
         }
     }

diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
@@ -1168,7 +1168,7 @@ public Collection<Object> createComponents(
             configPath,
             compatConfig
         );
-        dcf = new DynamicConfigFactory(cr, settings, configPath, localClient, threadPool, cih, passwordHasher);
+        dcf = new DynamicConfigFactory(cr, settings, configPath, localClient, threadPool, cih, passwordHasher, ar);
         dcf.registerDCFListener(backendRegistry);
         dcf.registerDCFListener(compatConfig);
         dcf.registerDCFListener(irr);

diff --git a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java
@@ -43,6 +43,7 @@ public class ApiTokenRepository implements ClusterStateListener {
     private Client client;
 
     void reloadApiTokensFromIndex() {
+        log.info("Reloading api tokens from index. Currnet entries: " + jtis.entrySet());
         try {
             jtis.clear();
             client.prepareSearch(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX)

diff --git a/src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java b/src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java
@@ -63,8 +63,8 @@ public class ApiTokenAuthenticator implements HTTPAuthenticator {
     private final ApiTokenRepository apiTokenRepository;
 
     @SuppressWarnings("removal")
-    @Inject
     public ApiTokenAuthenticator(Settings settings, String clusterName, ApiTokenRepository apiTokenRepository) {
+        log.info("We instantiating it");
         String apiTokenEnabledSetting = settings.get("enabled", "true");
         apiTokenEnabled = Boolean.parseBoolean(apiTokenEnabledSetting);
 
@@ -128,6 +128,7 @@ private AuthCredentials extractCredentials0(final SecurityRequest request, final
             log.error("Api token authentication is disabled");
             return null;
         }
+        log.info("API TOKEN AUTHENTICATOR IS BEING CALLED");
 
         String jwtToken = extractJwtFromHeader(request);
         if (jwtToken == null) {

diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluationContext.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluationContext.java
@@ -61,7 +61,6 @@ public class PrivilegesEvaluationContext {
      */
     private final Map<String, WildcardMatcher> renderedPatternTemplateCache = new HashMap<>();
 
-    @Inject
     public PrivilegesEvaluationContext(
         User user,
         ImmutableSet<String> mappedRoles,

diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java
@@ -157,7 +157,6 @@ public class PrivilegesEvaluator {
     private final AtomicReference<ActionPrivileges> actionPrivileges = new AtomicReference<>();
     private ApiTokenRepository apiTokenRepository;
 
-    @Inject
     public PrivilegesEvaluator(
         final ClusterService clusterService,
         Supplier<ClusterState> clusterStateSupplier,
@@ -221,6 +220,8 @@ public PrivilegesEvaluator(
             });
         }
 
+        this.apiTokenRepository = apiTokenRepository;
+
     }
 
     void updateConfiguration(

diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java
@@ -43,6 +43,7 @@
 import org.opensearch.client.Client;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.security.DefaultObjectMapper;
+import org.opensearch.security.action.apitokens.ApiTokenRepository;
 import org.opensearch.security.auditlog.config.AuditConfig;
 import org.opensearch.security.auth.internal.InternalAuthenticationBackend;
 import org.opensearch.security.configuration.ClusterInfoHolder;
@@ -127,6 +128,7 @@ public final static <T> SecurityDynamicConfiguration<T> addStatics(SecurityDynam
     private final Path configPath;
     private final InternalAuthenticationBackend iab;
     private final ClusterInfoHolder cih;
+    private final ApiTokenRepository ar;
 
     SecurityDynamicConfiguration<?> config;
 
@@ -137,14 +139,16 @@ public DynamicConfigFactory(
         Client client,
         ThreadPool threadPool,
         ClusterInfoHolder cih,
-        PasswordHasher passwordHasher
+        PasswordHasher passwordHasher,
+        ApiTokenRepository ar
     ) {
         super();
         this.cr = cr;
         this.opensearchSettings = opensearchSettings;
         this.configPath = configPath;
         this.cih = cih;
         this.iab = new InternalAuthenticationBackend(passwordHasher);
+        this.ar = ar;
 
         if (opensearchSettings.getAsBoolean(ConfigConstants.SECURITY_UNSUPPORTED_LOAD_STATIC_RESOURCES, true)) {
             try {
@@ -269,7 +273,7 @@ public void onChange(ConfigurationMap typeToConfig) {
         );
 
         // rebuild v7 Models
-        dcm = new DynamicConfigModelV7(getConfigV7(config), opensearchSettings, configPath, iab, this.cih);
+        dcm = new DynamicConfigModelV7(getConfigV7(config), opensearchSettings, configPath, iab, this.cih, ar);
         ium = new InternalUsersModelV7(internalusers, roles, rolesmapping);
         cm = new ConfigModelV7(roles, rolesmapping, actionGroups, tenants, dcm, opensearchSettings);
 

diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java
@@ -88,20 +88,23 @@ public class DynamicConfigModelV7 extends DynamicConfigModel {
     private List<ClientBlockRegistry<InetAddress>> ipClientBlockRegistries;
     private Multimap<String, ClientBlockRegistry<String>> authBackendClientBlockRegistries;
     private final ClusterInfoHolder cih;
+    private final ApiTokenRepository ar;
 
     public DynamicConfigModelV7(
         ConfigV7 config,
         Settings opensearchSettings,
         Path configPath,
         InternalAuthenticationBackend iab,
-        ClusterInfoHolder cih
+        ClusterInfoHolder cih,
+        ApiTokenRepository ar
     ) {
         super();
         this.config = config;
         this.opensearchSettings = opensearchSettings;
         this.configPath = configPath;
         this.iab = iab;
         this.cih = cih;
+        this.ar = ar;
         buildAAA();
     }
 
@@ -390,10 +393,11 @@ private void buildAAA() {
          * order: -2 - prioritize the Api token authentication when it gets enabled
          */
         Settings apiTokenSettings = getDynamicApiTokenSettings();
-        if (!isKeyNull(apiTokenSettings, "signing_key") && !isKeyNull(apiTokenSettings, "encryption_key")) {
+        log.info("APITOKENSETTINGS" + apiTokenSettings.toString());
+        if (!isKeyNull(apiTokenSettings, "signing_key")) {
             final AuthDomain _ad = new AuthDomain(
                 new NoOpAuthenticationBackend(Settings.EMPTY, null),
-                new ApiTokenAuthenticator(getDynamicApiTokenSettings(), this.cih.getClusterName(), apiTokenRepository),
+                new ApiTokenAuthenticator(getDynamicApiTokenSettings(), this.cih.getClusterName(), ar),
                 false,
                 -2
             );

diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java
@@ -517,8 +517,8 @@ public Boolean getEnabled() {
             return enabled;
         }
 
-        public void setEnabled(Boolean oboEnabled) {
-            this.enabled = oboEnabled;
+        public void setEnabled(Boolean apiTokensEnabled) {
+            this.enabled = apiTokensEnabled;
         }
 
         public String getSigningKey() {

diff --git a/src/test/java/org/opensearch/security/privileges/SystemIndexAccessEvaluatorTest.java b/src/test/java/org/opensearch/security/privileges/SystemIndexAccessEvaluatorTest.java
@@ -32,6 +32,7 @@
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.util.concurrent.ThreadContext;
+import org.opensearch.security.action.apitokens.ApiTokenRepository;
 import org.opensearch.security.auditlog.AuditLog;
 import org.opensearch.security.resolver.IndexResolverReplacer;
 import org.opensearch.security.resolver.IndexResolverReplacer.Resolved;
@@ -163,7 +164,8 @@ PrivilegesEvaluationContext ctx(String action) {
             null,
             null,
             indexNameExpressionResolver,
-            null
+            null,
+                mock(ApiTokenRepository.class)
         );
     }