Handle exceptions for adding request body audit log if rest request i…

…s invalid (opensearch-project#4232) Signed-off-by: Aayush Singhal <siaayush@amazon.com>
terryquigleysas · Apr 16, 2024 · 0bfe765 · 0bfe765
1 parent cb34962
commit 0bfe765
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 1 deletion.
diff --git a/src/main/java/org/opensearch/security/auditlog/impl/AuditMessage.java b/src/main/java/org/opensearch/security/auditlog/impl/AuditMessage.java
@@ -27,6 +27,8 @@
 import com.google.common.annotations.VisibleForTesting;
 import org.apache.commons.codec.digest.DigestUtils;
 import org.apache.hc.core5.net.URIBuilder;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 import org.opensearch.ExceptionsHelper;
 import org.opensearch.cluster.service.ClusterService;
@@ -59,6 +61,8 @@
 
 public final class AuditMessage {
 
+    private static final Logger log = LogManager.getLogger(AuditMessage.class);
+
     // clustername and cluster uuid
     private static final WildcardMatcher AUTHORIZATION_HEADER = WildcardMatcher.from("Authorization", false);
     private static final String SENSITIVE_KEY = "password";
@@ -417,8 +421,9 @@ void addRestRequestInfo(final SecurityRequest request, final AuditConfig.Filter
                     } else {
                         auditInfo.put(REQUEST_BODY, requestBody);
                     }
-                } catch (IOException e) {
+                } catch (Exception e) {
                     auditInfo.put(REQUEST_BODY, "ERROR: Unable to generate request body");
+                    log.error("Error while generating request body for audit log", e);
                 }
             }
         }

diff --git a/src/test/java/org/opensearch/security/auditlog/impl/AuditMessageTest.java b/src/test/java/org/opensearch/security/auditlog/impl/AuditMessageTest.java
@@ -26,9 +26,16 @@
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.collect.Tuple;
 import org.opensearch.common.xcontent.XContentType;
+import org.opensearch.core.common.bytes.BytesArray;
 import org.opensearch.core.common.bytes.BytesReference;
+import org.opensearch.core.xcontent.NamedXContentRegistry;
+import org.opensearch.http.HttpChannel;
+import org.opensearch.http.HttpRequest;
+import org.opensearch.rest.RestRequest;
 import org.opensearch.security.auditlog.AuditLog;
 import org.opensearch.security.auditlog.config.AuditConfig;
+import org.opensearch.security.filter.SecurityRequest;
+import org.opensearch.security.filter.SecurityRequestFactory;
 import org.opensearch.security.securityconf.impl.CType;
 
 import static org.junit.Assert.assertEquals;
@@ -155,4 +162,41 @@ public void testBCryptHashIsRedacted() {
         message.addSecurityConfigTupleToRequestBody(new Tuple<>(XContentType.JSON, ref), internalUsersDocId);
         assertEquals("Hash in tuple is __HASH__", message.getAsMap().get(AuditMessage.REQUEST_BODY));
     }
+
+    @Test
+    public void testRequestBodyLoggingWithInvalidSourceOrContentTypeParam() {
+        when(auditConfig.getFilter().shouldLogRequestBody()).thenReturn(true);
+
+        HttpRequest httpRequest = mock(HttpRequest.class);
+
+        // No content or Source paramater
+        when(httpRequest.uri()).thenReturn("");
+        when(httpRequest.content()).thenReturn(new BytesArray(new byte[0]));
+
+        RestRequest restRequest = RestRequest.request(mock(NamedXContentRegistry.class), httpRequest, mock(HttpChannel.class));
+        SecurityRequest request = SecurityRequestFactory.from(restRequest);
+
+        message.addRestRequestInfo(request, auditConfig.getFilter());
+        assertNull(message.getAsMap().get(AuditMessage.REQUEST_BODY));
+
+        // No source parameter, content present but Invalid content-type header
+        when(httpRequest.uri()).thenReturn("");
+        when(httpRequest.content()).thenReturn(new BytesArray(new byte[1]));
+
+        restRequest = RestRequest.request(mock(NamedXContentRegistry.class), httpRequest, mock(HttpChannel.class));
+        request = SecurityRequestFactory.from(restRequest);
+
+        message.addRestRequestInfo(request, auditConfig.getFilter());
+        assertEquals("ERROR: Unable to generate request body", message.getAsMap().get(AuditMessage.REQUEST_BODY));
+
+        // No content, source parameter present but Invalid source-content-type parameter
+        when(httpRequest.uri()).thenReturn("/aaaa?source=request_body");
+        when(httpRequest.content()).thenReturn(new BytesArray(new byte[0]));
+
+        restRequest = RestRequest.request(mock(NamedXContentRegistry.class), httpRequest, mock(HttpChannel.class));
+        request = SecurityRequestFactory.from(restRequest);
+
+        message.addRestRequestInfo(request, auditConfig.getFilter());
+        assertEquals("ERROR: Unable to generate request body", message.getAsMap().get(AuditMessage.REQUEST_BODY));
+    }
 }