Fix all tests

Signed-off-by: Peter Nied <[email protected]>
peternied · Sep 29, 2023 · 26c6b83 · 26c6b83
1 parent da8d0cb
commit 26c6b83
Show file tree

Hide file tree

Showing 23 changed files with 135 additions and 135 deletions.
diff --git a/src/main/java/com/amazon/dlic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java b/src/main/java/com/amazon/dlic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java
@@ -150,10 +150,10 @@ protected String getJwtTokenString(SecurityRequest request) {
 
         if (jwtUrlParameter != null) {
             if (jwtToken == null || jwtToken.isEmpty()) {
-                jwtToken = request.param(jwtUrlParameter);
+                jwtToken = request.params().get(jwtUrlParameter);
             } else {
                 // just consume to avoid "contains unrecognized parameter"
-                request.param(jwtUrlParameter);
+                request.params().get(jwtUrlParameter);
             }
         }
 

diff --git a/src/main/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticator.java b/src/main/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticator.java
@@ -114,10 +114,10 @@ private AuthCredentials extractCredentials0(final SecurityRequest request) {
         }
 
         if ((jwtToken == null || jwtToken.isEmpty()) && jwtUrlParameter != null) {
-            jwtToken = request.param(jwtUrlParameter);
+            jwtToken = request.params().get(jwtUrlParameter);
         } else {
             // just consume to avoid "contains unrecognized parameter"
-            request.param(jwtUrlParameter);
+            request.params().get(jwtUrlParameter);
         }
 
         if (jwtToken == null || jwtToken.length() == 0) {

diff --git a/src/main/java/org/opensearch/security/auditlog/AuditLog.java b/src/main/java/org/opensearch/security/auditlog/AuditLog.java
@@ -45,22 +45,14 @@
 public interface AuditLog extends Closeable {
 
     // login
-    void logFailedLogin(String effectiveUser, boolean securityadmin, String initiatingUser, RestRequest request);
+    void logFailedLogin(String effectiveUser, boolean securityadmin, String initiatingUser, SecurityRequest request);
 
-    default void logFailedLogin(String effectiveUser, boolean securityadmin, String initiatingUser, SecurityRequest request) {
-        this.logFailedLogin(effectiveUser, securityadmin, initiatingUser, request.asRestRequest().get());
-    }
-
-    void logSucceededLogin(String effectiveUser, boolean securityadmin, String initiatingUser, RestRequest request);
-
-    default void logSucceededLogin(String effectiveUser, boolean securityadmin, String initiatingUser, SecurityRequest request) {
-        logSucceededLogin(effectiveUser, securityadmin, initiatingUser, request.asRestRequest().get());
-    }
+    void logSucceededLogin(String effectiveUser, boolean securityadmin, String initiatingUser, SecurityRequest request);
 
     // privs
-    void logMissingPrivileges(String privilege, String effectiveUser, RestRequest request);
+    void logMissingPrivileges(String privilege, String effectiveUser, SecurityRequest request);
 
-    void logGrantedPrivileges(String effectiveUser, RestRequest request);
+    void logGrantedPrivileges(String effectiveUser, SecurityRequest request);
 
     void logMissingPrivileges(String privilege, TransportRequest request, Task task);
 
@@ -72,21 +64,13 @@ default void logSucceededLogin(String effectiveUser, boolean securityadmin, Stri
     // spoof
     void logBadHeaders(TransportRequest request, String action, Task task);
 
-    void logBadHeaders(RestRequest request);
-
-    default void logBadHeaders(SecurityRequest request) {
-        this.logBadHeaders(request.asRestRequest().get());
-    }
+    void logBadHeaders(SecurityRequest request);
 
     void logSecurityIndexAttempt(TransportRequest request, String action, Task task);
 
     void logSSLException(TransportRequest request, Throwable t, String action, Task task);
 
-    void logSSLException(RestRequest request, Throwable t);
-
-    default void logSSLException(SecurityRequest request, Throwable t) {
-        this.logSSLException(request.asRestRequest().get(), t);
-    }
+    void logSSLException(SecurityRequest request, Throwable t);
 
     void logDocumentRead(String index, String id, ShardId shardId, Map<String, String> fieldNameValues);
 

diff --git a/src/main/java/org/opensearch/security/auditlog/AuditLogSslExceptionHandler.java b/src/main/java/org/opensearch/security/auditlog/AuditLogSslExceptionHandler.java
@@ -28,6 +28,7 @@
 
 import org.opensearch.OpenSearchException;
 import org.opensearch.rest.RestRequest;
+import org.opensearch.security.filter.SecurityRequest;
 import org.opensearch.security.ssl.SslExceptionHandler;
 import org.opensearch.tasks.Task;
 import org.opensearch.transport.TransportRequest;
@@ -42,7 +43,7 @@ public AuditLogSslExceptionHandler(final AuditLog auditLog) {
     }
 
     @Override
-    public void logError(Throwable t, RestRequest request, int type) {
+    public void logError(Throwable t, SecurityRequest request, int type) {
         switch (type) {
             case 0:
                 auditLog.logSSLException(request, t);
@@ -58,7 +59,7 @@ public void logError(Throwable t, RestRequest request, int type) {
     @Override
     public void logError(Throwable t, boolean isRest) {
         if (isRest) {
-            auditLog.logSSLException((RestRequest)null, t);
+            auditLog.logSSLException(null, t);
         } else {
             auditLog.logSSLException(null, t, null, null);
         }

diff --git a/src/main/java/org/opensearch/security/auditlog/NullAuditLog.java b/src/main/java/org/opensearch/security/auditlog/NullAuditLog.java
@@ -38,6 +38,7 @@
 import org.opensearch.rest.RestRequest;
 import org.opensearch.security.auditlog.config.AuditConfig;
 import org.opensearch.security.compliance.ComplianceConfig;
+import org.opensearch.security.filter.SecurityRequest;
 import org.opensearch.tasks.Task;
 import org.opensearch.transport.TransportRequest;
 
@@ -49,12 +50,12 @@ public void close() throws IOException {
     }
 
     @Override
-    public void logFailedLogin(String effectiveUser, boolean securityadmin, String initiatingUser, RestRequest request) {
+    public void logFailedLogin(String effectiveUser, boolean securityadmin, String initiatingUser, SecurityRequest request) {
         // noop, intentionally left empty
     }
 
     @Override
-    public void logSucceededLogin(String effectiveUser, boolean securityadmin, String initiatingUser, RestRequest request) {
+    public void logSucceededLogin(String effectiveUser, boolean securityadmin, String initiatingUser, SecurityRequest request) {
         // noop, intentionally left empty
     }
 
@@ -79,7 +80,7 @@ public void logBadHeaders(TransportRequest request, String action, Task task) {
     }
 
     @Override
-    public void logBadHeaders(RestRequest request) {
+    public void logBadHeaders(SecurityRequest request) {
         // noop, intentionally left empty
     }
 
@@ -94,17 +95,17 @@ public void logSSLException(TransportRequest request, Throwable t, String action
     }
 
     @Override
-    public void logSSLException(RestRequest request, Throwable t) {
+    public void logSSLException(SecurityRequest request, Throwable t) {
         // noop, intentionally left empty
     }
 
     @Override
-    public void logMissingPrivileges(String privilege, String effectiveUser, RestRequest request) {
+    public void logMissingPrivileges(String privilege, String effectiveUser, SecurityRequest request) {
         // noop, intentionally left empty
     }
 
     @Override
-    public void logGrantedPrivileges(String effectiveUser, RestRequest request) {
+    public void logGrantedPrivileges(String effectiveUser, SecurityRequest request) {
         // noop, intentionally left empty
     }
 

diff --git a/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java b/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java
@@ -66,6 +66,7 @@
 import org.opensearch.security.auditlog.config.AuditConfig;
 import org.opensearch.security.compliance.ComplianceConfig;
 import org.opensearch.security.dlic.rest.support.Utils;
+import org.opensearch.security.filter.SecurityRequest;
 import org.opensearch.security.support.Base64Helper;
 import org.opensearch.security.support.ConfigConstants;
 import org.opensearch.security.user.User;
@@ -139,7 +140,7 @@ public ComplianceConfig getComplianceConfig() {
     }
 
     @Override
-    public void logFailedLogin(String effectiveUser, boolean securityadmin, String initiatingUser, RestRequest request) {
+    public void logFailedLogin(String effectiveUser, boolean securityadmin, String initiatingUser, SecurityRequest request) {
 
         if (!checkRestFilter(AuditCategory.FAILED_LOGIN, effectiveUser, request)) {
             return;
@@ -157,7 +158,7 @@ public void logFailedLogin(String effectiveUser, boolean securityadmin, String i
     }
 
     @Override
-    public void logSucceededLogin(String effectiveUser, boolean securityadmin, String initiatingUser, RestRequest request) {
+    public void logSucceededLogin(String effectiveUser, boolean securityadmin, String initiatingUser, SecurityRequest request) {
 
         if (!checkRestFilter(AuditCategory.AUTHENTICATED, effectiveUser, request)) {
             return;
@@ -174,7 +175,7 @@ public void logSucceededLogin(String effectiveUser, boolean securityadmin, Strin
     }
 
     @Override
-    public void logMissingPrivileges(String privilege, String effectiveUser, RestRequest request) {
+    public void logMissingPrivileges(String privilege, String effectiveUser, SecurityRequest request) {
         if (!checkRestFilter(AuditCategory.MISSING_PRIVILEGES, effectiveUser, request)) {
             return;
         }
@@ -189,7 +190,7 @@ public void logMissingPrivileges(String privilege, String effectiveUser, RestReq
     }
 
     @Override
-    public void logGrantedPrivileges(String effectiveUser, RestRequest request) {
+    public void logGrantedPrivileges(String effectiveUser, SecurityRequest request) {
         if (!checkRestFilter(AuditCategory.GRANTED_PRIVILEGES, effectiveUser, request)) {
             return;
         }
@@ -348,7 +349,7 @@ public void logBadHeaders(TransportRequest request, String action, Task task) {
     }
 
     @Override
-    public void logBadHeaders(RestRequest request) {
+    public void logBadHeaders(SecurityRequest request) {
 
         if (!checkRestFilter(AuditCategory.BAD_HEADERS, getUser(), request)) {
             return;
@@ -437,7 +438,7 @@ public void logSSLException(TransportRequest request, Throwable t, String action
     }
 
     @Override
-    public void logSSLException(RestRequest request, Throwable t) {
+    public void logSSLException(SecurityRequest request, Throwable t) {
 
         if (!checkRestFilter(AuditCategory.SSL_EXCEPTION, getUser(), request)) {
             return;
@@ -896,7 +897,7 @@ private boolean checkComplianceFilter(
     }
 
     @VisibleForTesting
-    boolean checkRestFilter(final AuditCategory category, final String effectiveUser, RestRequest request) {
+    boolean checkRestFilter(final AuditCategory category, final String effectiveUser, SecurityRequest request) {
         final boolean isTraceEnabled = log.isTraceEnabled();
         if (isTraceEnabled) {
             log.trace(

diff --git a/src/main/java/org/opensearch/security/auditlog/impl/AuditLogImpl.java b/src/main/java/org/opensearch/security/auditlog/impl/AuditLogImpl.java
@@ -34,6 +34,7 @@
 import org.opensearch.rest.RestRequest;
 import org.opensearch.security.auditlog.config.AuditConfig;
 import org.opensearch.security.auditlog.routing.AuditMessageRouter;
+import org.opensearch.security.filter.SecurityRequest;
 import org.opensearch.tasks.Task;
 import org.opensearch.threadpool.ThreadPool;
 import org.opensearch.transport.TransportRequest;
@@ -131,28 +132,28 @@ protected void save(final AuditMessage msg) {
     }
 
     @Override
-    public void logFailedLogin(String effectiveUser, boolean securityAdmin, String initiatingUser, RestRequest request) {
+    public void logFailedLogin(String effectiveUser, boolean securityAdmin, String initiatingUser, SecurityRequest request) {
         if (enabled) {
             super.logFailedLogin(effectiveUser, securityAdmin, initiatingUser, request);
         }
     }
 
     @Override
-    public void logSucceededLogin(String effectiveUser, boolean securityAdmin, String initiatingUser, RestRequest request) {
+    public void logSucceededLogin(String effectiveUser, boolean securityAdmin, String initiatingUser, SecurityRequest request) {
         if (enabled) {
             super.logSucceededLogin(effectiveUser, securityAdmin, initiatingUser, request);
         }
     }
 
     @Override
-    public void logMissingPrivileges(String privilege, String effectiveUser, RestRequest request) {
+    public void logMissingPrivileges(String privilege, String effectiveUser, SecurityRequest request) {
         if (enabled) {
             super.logMissingPrivileges(privilege, effectiveUser, request);
         }
     }
 
     @Override
-    public void logGrantedPrivileges(String effectiveUser, RestRequest request) {
+    public void logGrantedPrivileges(String effectiveUser, SecurityRequest request) {
         if (enabled) {
             super.logGrantedPrivileges(effectiveUser, request);
         }
@@ -187,7 +188,7 @@ public void logBadHeaders(TransportRequest request, String action, Task task) {
     }
 
     @Override
-    public void logBadHeaders(RestRequest request) {
+    public void logBadHeaders(SecurityRequest request) {
         if (enabled) {
             super.logBadHeaders(request);
         }
@@ -208,7 +209,7 @@ public void logSSLException(TransportRequest request, Throwable t, String action
     }
 
     @Override
-    public void logSSLException(RestRequest request, Throwable t) {
+    public void logSSLException(SecurityRequest request, Throwable t) {
         if (enabled) {
             super.logSSLException(request, t);
         }

diff --git a/src/main/java/org/opensearch/security/auditlog/impl/AuditMessage.java b/src/main/java/org/opensearch/security/auditlog/impl/AuditMessage.java
@@ -48,6 +48,7 @@
 import org.opensearch.security.auditlog.AuditLog.Origin;
 import org.opensearch.security.auditlog.config.AuditConfig;
 import org.opensearch.security.dlic.rest.support.Utils;
+import org.opensearch.security.filter.SecurityRequest;
 import org.opensearch.security.securityconf.impl.CType;
 import org.opensearch.security.support.WildcardMatcher;
 
@@ -369,16 +370,17 @@ void addRestMethod(final RestRequest.Method method) {
         }
     }
 
-    void addRestRequestInfo(final RestRequest request, final AuditConfig.Filter filter) {
+    void addRestRequestInfo(final SecurityRequest request, final AuditConfig.Filter filter) {
         if (request != null) {
-            final String path = request.path();
+            final String path = request.path().toString();
             addPath(path);
             addRestHeaders(request.getHeaders(), filter.shouldExcludeSensitiveHeaders());
             addRestParams(request.params());
             addRestMethod(request.method());
-            if (filter.shouldLogRequestBody() && request.hasContentOrSourceParam()) {
+
+            if (filter.shouldLogRequestBody() && request.asRestRequest().isPresent() && request.asRestRequest().get().hasContentOrSourceParam()) {
                 try {
-                    final Tuple<MediaType, BytesReference> xContentTuple = request.contentOrSourceParam();
+                    final Tuple<MediaType, BytesReference> xContentTuple = request.asRestRequest().get().contentOrSourceParam();
                     final String requestBody = XContentHelper.convertToJson(xContentTuple.v2(), false, xContentTuple.v1());
                     if (path != null
                         && requestBody != null

diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/AbstractApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/AbstractApiAction.java
@@ -49,6 +49,8 @@
 import org.opensearch.security.dlic.rest.validation.EndpointValidator;
 import org.opensearch.security.dlic.rest.validation.RequestContentValidator;
 import org.opensearch.security.dlic.rest.validation.ValidationResult;
+import org.opensearch.security.filter.SecurityRequest;
+import org.opensearch.security.filter.SecurityRequestFactory;
 import org.opensearch.security.securityconf.DynamicConfigFactory;
 import org.opensearch.security.securityconf.impl.CType;
 import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
@@ -535,6 +537,9 @@ public void onFailure(Exception e) {
     @Override
     protected final RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {
 
+        // Fix channel ordering
+        final SecurityRequest securityRequest = SecurityRequestFactory.from(request, null);
+
         // consume all parameters first so we can return a correct HTTP status,
         // not 400
         consumeParameters(request);
@@ -551,12 +556,12 @@ protected final RestChannelConsumer prepareRequest(RestRequest request, NodeClie
         final String userName = user == null ? null : user.getName();
         if (authError != null) {
             LOGGER.error("No permission to access REST API: " + authError);
-            securityApiDependencies.auditLog().logMissingPrivileges(authError, userName, request);
+            securityApiDependencies.auditLog().logMissingPrivileges(authError, userName, securityRequest);
             // for rest request
             request.params().clear();
             return channel -> forbidden(channel, "No permission to access REST API: " + authError);
         } else {
-            securityApiDependencies.auditLog().logGrantedPrivileges(userName, request);
+            securityApiDependencies.auditLog().logGrantedPrivileges(userName, securityRequest);
         }
 
         final var originalUserAndRemoteAddress = Utils.userAndRemoteAddressFrom(threadPool.getThreadContext());

diff --git a/src/main/java/org/opensearch/security/filter/SecurityRequest.java b/src/main/java/org/opensearch/security/filter/SecurityRequest.java
@@ -20,7 +20,7 @@ public interface SecurityRequest {
 
     public RestChannel getRestChannel();
 
-    public CharSequence path();
+    public String path();
 
     public Method method();
 
@@ -41,7 +41,5 @@ default public String header(final String headerName) {
             .orElse(null);
     }
 
-    public boolean paramAsBoolean(String string, boolean b);
-
-    public String param(String jwtUrlParameter);
+    public Map<String, String> params();
 }