Address review comments

Signed-off-by: Craig Perkins <cwperx@amazon.com>
opensearch-project · Dec 15, 2022 · 32d8e75 · 32d8e75
1 parent 134bcb0
commit 32d8e75
Show file tree

Hide file tree

Showing 7 changed files with 17 additions and 61 deletions.
diff --git a/...libs/authn/src/main/java/org/opensearch/authn/internal/InternalAuthenticationManager.java b/...libs/authn/src/main/java/org/opensearch/authn/internal/InternalAuthenticationManager.java
@@ -32,9 +32,6 @@ public class InternalAuthenticationManager implements AuthenticationManager {
      * and this instantiation uses the default security manager
      */
     public InternalAuthenticationManager() {
-        // final SecurityManager securityManager = new DefaultSecurityManager(InternalRealm.INSTANCE);
-        // SecurityUtils.setSecurityManager(securityManager);
-
         Factory<SecurityManager> factory = new IniSecurityManagerFactory("classpath:shiro.ini");
         SecurityManager securityManager = factory.getInstance();
         SecurityUtils.setSecurityManager(securityManager);

diff --git a/...y/src/internalClusterTest/java/org/opensearch/identity/HttpSmokeTestCaseWithIdentity.java b/...y/src/internalClusterTest/java/org/opensearch/identity/HttpSmokeTestCaseWithIdentity.java
@@ -21,6 +21,12 @@
 import java.util.Arrays;
 import java.util.Collection;
 
+/**
+ * Abstract Rest Test Case for IdentityPlugin that installs and enables IdentityPlugin and removes mock
+ * http transport to enable REST requests against a test cluster
+ *
+ * @opensearch.experimental
+ */
 // TODO not sure why ThreadLeakScope.NONE is required
 @ThreadLeakScope(ThreadLeakScope.Scope.NONE)
 public abstract class HttpSmokeTestCaseWithIdentity extends OpenSearchIntegTestCase {

diff --git a/...x/modules/identity/src/main/java/org/opensearch/identity/PrivilegesEvaluatorResponse.java b/...x/modules/identity/src/main/java/org/opensearch/identity/PrivilegesEvaluatorResponse.java
@@ -8,55 +8,19 @@
 
 package org.opensearch.identity;
 
-import org.opensearch.action.admin.indices.create.CreateIndexRequestBuilder;
-
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Set;
-
+/**
+ * Response for privilege evaluation that determines if a request to perform an action is permitted
+ *
+ * @opensearch.experimental
+ */
 public class PrivilegesEvaluatorResponse {
     boolean allowed = false;
-    Set<String> missingPrivileges = new HashSet<String>();
-    Set<String> missingSecurityRoles = new HashSet<>();
-    Set<String> resolvedSecurityRoles = new HashSet<>();
-    Map<String, Set<String>> allowedFlsFields;
-    Map<String, Set<String>> maskedFields;
-    Map<String, Set<String>> queries;
     PrivilegesEvaluatorResponseState state = PrivilegesEvaluatorResponseState.PENDING;
-    CreateIndexRequestBuilder createIndexRequestBuilder;
 
     public boolean isAllowed() {
         return allowed;
     }
 
-    public Set<String> getMissingPrivileges() {
-        return new HashSet<String>(missingPrivileges);
-    }
-
-    public Set<String> getMissingSecurityRoles() {
-        return new HashSet<>(missingSecurityRoles);
-    }
-
-    public Set<String> getResolvedSecurityRoles() {
-        return new HashSet<>(resolvedSecurityRoles);
-    }
-
-    public Map<String, Set<String>> getAllowedFlsFields() {
-        return allowedFlsFields;
-    }
-
-    public Map<String, Set<String>> getMaskedFields() {
-        return maskedFields;
-    }
-
-    public Map<String, Set<String>> getQueries() {
-        return queries;
-    }
-
-    public CreateIndexRequestBuilder getCreateIndexRequestBuilder() {
-        return createIndexRequestBuilder;
-    }
-
     public PrivilegesEvaluatorResponse markComplete() {
         this.state = PrivilegesEvaluatorResponseState.COMPLETE;
         return this;
@@ -77,17 +41,7 @@ public boolean isPending() {
 
     @Override
     public String toString() {
-        return "PrivEvalResponse [allowed="
-            + allowed
-            + ", missingPrivileges="
-            + missingPrivileges
-            + ", allowedFlsFields="
-            + allowedFlsFields
-            + ", maskedFields="
-            + maskedFields
-            + ", queries="
-            + queries
-            + "]";
+        return "PrivEvalResponse [allowed=" + allowed + "]";
     }
 
     public static enum PrivilegesEvaluatorResponseState {

diff --git a/sandbox/modules/identity/src/main/java/org/opensearch/identity/SecurityFilter.java b/sandbox/modules/identity/src/main/java/org/opensearch/identity/SecurityFilter.java
@@ -77,7 +77,7 @@ private <Request extends ActionRequest, Response extends ActionResponse> void ap
                     + ", action="
                     + action
                     + " apply0)";
-                log.info(prefix + " Access token provided " + encodedJwt);
+                log.debug(prefix + " Access token provided " + encodedJwt);
             } else {
                 // TODO Figure out where internal actions are invoked and create token on invocation
                 // No token provided, may be an internal request

diff --git a/sandbox/modules/identity/src/main/java/org/opensearch/identity/SecurityRestFilter.java b/sandbox/modules/identity/src/main/java/org/opensearch/identity/SecurityRestFilter.java
@@ -88,7 +88,7 @@ private boolean checkAndAuthenticateRequest(RestRequest request, RestChannel cha
                 + ", jwtClaims="
                 + jwtClaims
                 + " checkAndAuthenticateRequest)";
-            log.info(prefix + " Created internal access token " + encodedJwt);
+            log.debug(prefix + " Created internal access token " + encodedJwt);
             threadContext.putHeader(ThreadContextConstants.OPENSEARCH_AUTHENTICATION_TOKEN_HEADER, encodedJwt);
         }
         return true;

diff --git a/sandbox/modules/identity/src/test/java/org/opensearch/identity/BasicAuthTests.java b/sandbox/modules/identity/src/test/java/org/opensearch/identity/BasicAuthTests.java
@@ -16,6 +16,8 @@
 
 import java.nio.charset.StandardCharsets;
 
+import static org.hamcrest.Matchers.containsString;
+
 @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 1)
 public class BasicAuthTests extends AbstractIdentityTestCase {
     public void testBasicAuthSuccess() throws Exception {
@@ -28,7 +30,7 @@ public void testBasicAuthSuccess() throws Exception {
         String content = new String(response.getEntity().getContent().readAllBytes(), StandardCharsets.UTF_8);
 
         assertEquals(RestStatus.OK.getStatus(), response.getStatusLine().getStatusCode());
-        assertTrue(content.contains("\"status\":\"green\""));
+        assertThat(content, containsString("green"));
     }
 
     public void testBasicAuthUnauthorized() throws Exception {

diff --git a/server/src/main/java/org/opensearch/rest/RestController.java b/server/src/main/java/org/opensearch/rest/RestController.java
@@ -86,9 +86,6 @@ public class RestController implements HttpServerTransport.Dispatcher {
     private static final Logger logger = LogManager.getLogger(RestController.class);
     private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestController.class);
     private static final String OPENSEARCH_PRODUCT_ORIGIN_HTTP_HEADER = "X-opensearch-product-origin";
-
-    private static final String OPENSEARCH_AUTHENTICATION_TOKEN_HEADER = "_opensearch_auth_token";
-
     private static final BytesReference FAVICON_RESPONSE;
 
     static {