[Backport 2.11] Use BytesRestResponse constructor with contentType in…

… asRestResponse (#3717) (#3721) Backport #3717 to 2.11 Signed-off-by: Craig Perkins <cwperx@amazon.com>
opensearch-project · Nov 16, 2023 · 2dab119 · 2dab119
1 parent 1d46ba6
commit 2dab119
Show file tree

Hide file tree

Showing 9 changed files with 231 additions and 23 deletions.
diff --git a/src/main/java/com/amazon/dlic/auth/http/kerberos/HTTPSpnegoAuthenticator.java b/src/main/java/com/amazon/dlic/auth/http/kerberos/HTTPSpnegoAuthenticator.java
@@ -52,6 +52,7 @@
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.util.concurrent.ThreadContext;
 import org.opensearch.common.xcontent.XContentFactory;
+import org.opensearch.common.xcontent.XContentType;
 import org.opensearch.core.xcontent.XContentBuilder;
 import org.opensearch.env.Environment;
 import org.opensearch.security.auth.HTTPAuthenticator;
@@ -286,10 +287,12 @@ public GSSCredential run() throws GSSException {
     public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest request, AuthCredentials creds) {
         final Map<String, String> headers = new HashMap<>();
         String responseBody = "";
+        String contentType = null;
+        SecurityResponse response;
         final String negotiateResponseBody = getNegotiateResponseBody();
         if (negotiateResponseBody != null) {
             responseBody = negotiateResponseBody;
-            headers.putAll(SecurityResponse.CONTENT_TYPE_APP_JSON);
+            contentType = XContentType.JSON.mediaType();
         }
 
         if (creds == null || creds.getNativeCredentials() == null) {
@@ -298,7 +301,12 @@ public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest
             headers.put("WWW-Authenticate", "Negotiate " + Base64.getEncoder().encodeToString((byte[]) creds.getNativeCredentials()));
         }
 
-        return Optional.of(new SecurityResponse(SC_UNAUTHORIZED, headers, responseBody));
+        if (contentType != null) {
+            response = new SecurityResponse(SC_UNAUTHORIZED, headers, responseBody, contentType);
+        } else {
+            response = new SecurityResponse(SC_UNAUTHORIZED, headers, responseBody);
+        }
+        return Optional.of(response);
     }
 
     @Override

diff --git a/src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java b/src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java
@@ -236,10 +236,10 @@ private Optional<SecurityResponse> handleLowLevel(RestRequest restRequest) throw
 
             String responseBodyString = DefaultObjectMapper.objectMapper.writeValueAsString(responseBody);
 
-            return Optional.of(new SecurityResponse(HttpStatus.SC_OK, SecurityResponse.CONTENT_TYPE_APP_JSON, responseBodyString));
+            return Optional.of(new SecurityResponse(HttpStatus.SC_OK, null, responseBodyString, XContentType.JSON.mediaType()));
         } catch (JsonProcessingException e) {
             log.warn("Error while parsing JSON for /_opendistro/_security/api/authtoken", e);
-            return Optional.of(new SecurityResponse(HttpStatus.SC_BAD_REQUEST, new Exception("JSON could not be parsed")));
+            return Optional.of(new SecurityResponse(HttpStatus.SC_BAD_REQUEST, "JSON could not be parsed"));
         }
     }
 

diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java
@@ -203,7 +203,7 @@ public boolean authenticate(final SecurityRequestChannel request) {
                 log.debug("Rejecting REST request because of blocked address: {}", request.getRemoteAddress().orElse(null));
             }
 
-            request.queueForSending(new SecurityResponse(SC_UNAUTHORIZED, new Exception("Authentication finally failed")));
+            request.queueForSending(new SecurityResponse(SC_UNAUTHORIZED, "Authentication finally failed"));
             return false;
         }
 
@@ -225,7 +225,7 @@ public boolean authenticate(final SecurityRequestChannel request) {
 
         if (!isInitialized()) {
             log.error("Not yet initialized (you may need to run securityadmin)");
-            request.queueForSending(new SecurityResponse(SC_SERVICE_UNAVAILABLE, new Exception("OpenSearch Security not initialized.")));
+            request.queueForSending(new SecurityResponse(SC_SERVICE_UNAVAILABLE, "OpenSearch Security not initialized."));
             return false;
         }
 
@@ -355,11 +355,7 @@ public boolean authenticate(final SecurityRequestChannel request) {
                 log.error("Cannot authenticate rest user because admin user is not permitted to login via HTTP");
                 auditLog.logFailedLogin(authenticatedUser.getName(), true, null, request);
                 request.queueForSending(
-                    new SecurityResponse(
-                        SC_FORBIDDEN,
-                        null,
-                        "Cannot authenticate user because admin user is not permitted to login via HTTP"
-                    )
+                    new SecurityResponse(SC_FORBIDDEN, "Cannot authenticate user because admin user is not permitted to login via HTTP")
                 );
                 return false;
             }
@@ -430,7 +426,7 @@ public boolean authenticate(final SecurityRequestChannel request) {
             notifyIpAuthFailureListeners(request, authCredentials);
 
             request.queueForSending(
-                challengeResponse.orElseGet(() -> new SecurityResponse(SC_UNAUTHORIZED, null, "Authentication finally failed"))
+                challengeResponse.orElseGet(() -> new SecurityResponse(SC_UNAUTHORIZED, "Authentication finally failed"))
             );
             return false;
         }

diff --git a/src/main/java/org/opensearch/security/filter/SecurityResponse.java b/src/main/java/org/opensearch/security/filter/SecurityResponse.java
@@ -12,10 +12,14 @@
 package org.opensearch.security.filter;
 
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 import org.apache.http.HttpHeaders;
 import org.opensearch.common.xcontent.XContentFactory;
+import org.opensearch.common.xcontent.XContentType;
 import org.opensearch.core.rest.RestStatus;
 import org.opensearch.rest.BytesRestResponse;
 import org.opensearch.rest.RestResponse;
@@ -25,26 +29,63 @@ public class SecurityResponse {
     public static final Map<String, String> CONTENT_TYPE_APP_JSON = Map.of(HttpHeaders.CONTENT_TYPE, "application/json");
 
     private final int status;
-    private final Map<String, String> headers;
+    private Map<String, List<String>> headers;
     private final String body;
+    private final String contentType;
 
     public SecurityResponse(final int status, final Exception e) {
         this.status = status;
-        this.headers = CONTENT_TYPE_APP_JSON;
+        populateHeaders(CONTENT_TYPE_APP_JSON);
         this.body = generateFailureMessage(e);
+        this.contentType = XContentType.JSON.mediaType();
+    }
+
+    public SecurityResponse(final int status, String body) {
+        this.status = status;
+        this.body = body;
+        this.contentType = null;
     }
 
     public SecurityResponse(final int status, final Map<String, String> headers, final String body) {
         this.status = status;
-        this.headers = headers;
+        populateHeaders(headers);
+        this.body = body;
+        this.contentType = null;
+    }
+
+    public SecurityResponse(final int status, final Map<String, String> headers, final String body, String contentType) {
+        this.status = status;
         this.body = body;
+        this.contentType = contentType;
+        populateHeaders(headers);
+    }
+
+    private void populateHeaders(Map<String, String> headers) {
+        if (headers != null) {
+            headers.entrySet().forEach(entry -> addHeader(entry.getKey(), entry.getValue()));
+        }
+    }
+
+    /**
+     * Add a custom header.
+     */
+    public void addHeader(String name, String value) {
+        if (headers == null) {
+            headers = new HashMap<>(2);
+        }
+        List<String> header = headers.get(name);
+        if (header == null) {
+            header = new ArrayList<>();
+            headers.put(name, header);
+        }
+        header.add(value);
     }
 
     public int getStatus() {
         return status;
     }
 
-    public Map<String, String> getHeaders() {
+    public Map<String, List<String>> getHeaders() {
         return headers;
     }
 
@@ -53,9 +94,14 @@ public String getBody() {
     }
 
     public RestResponse asRestResponse() {
-        final RestResponse restResponse = new BytesRestResponse(RestStatus.fromCode(getStatus()), getBody());
+        final RestResponse restResponse;
+        if (this.contentType != null) {
+            restResponse = new BytesRestResponse(RestStatus.fromCode(getStatus()), this.contentType, getBody());
+        } else {
+            restResponse = new BytesRestResponse(RestStatus.fromCode(getStatus()), getBody());
+        }
         if (getHeaders() != null) {
-            getHeaders().forEach(restResponse::addHeader);
+            getHeaders().entrySet().forEach(entry -> { entry.getValue().forEach(value -> restResponse.addHeader(entry.getKey(), value)); });
         }
         return restResponse;
     }

diff --git a/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java b/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java
@@ -233,7 +233,7 @@ void authorizeRequest(RestHandler original, SecurityRequestChannel request, User
                 }
                 log.debug(err);
 
-                request.queueForSending(new SecurityResponse(HttpStatus.SC_UNAUTHORIZED, null, err));
+                request.queueForSending(new SecurityResponse(HttpStatus.SC_UNAUTHORIZED, err));
                 return;
             }
         }
@@ -276,7 +276,7 @@ public void checkAndAuthenticateRequest(SecurityRequestChannel requestChannel) t
         } catch (SSLPeerUnverifiedException e) {
             log.error("No ssl info", e);
             auditLog.logSSLException(requestChannel, e);
-            requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, new Exception("No ssl info")));
+            requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, e));
             return;
         }
 

diff --git a/src/main/java/org/opensearch/security/securityconf/impl/AllowlistingSettings.java b/src/main/java/org/opensearch/security/securityconf/impl/AllowlistingSettings.java
@@ -19,6 +19,7 @@
 
 import org.apache.http.HttpStatus;
 import org.opensearch.common.xcontent.XContentFactory;
+import org.opensearch.common.xcontent.XContentType;
 import org.opensearch.core.rest.RestStatus;
 import org.opensearch.security.filter.SecurityRequest;
 import org.opensearch.security.filter.SecurityResponse;
@@ -112,7 +113,7 @@ public Optional<SecurityResponse> checkRequestIsAllowed(final SecurityRequest re
         // if allowlisting is enabled but the request is not allowlisted, then return false, otherwise true.
         if (this.enabled && !requestIsAllowlisted(request)) {
             return Optional.of(
-                new SecurityResponse(HttpStatus.SC_FORBIDDEN, SecurityResponse.CONTENT_TYPE_APP_JSON, generateFailureMessage(request))
+                new SecurityResponse(HttpStatus.SC_FORBIDDEN, null, generateFailureMessage(request), XContentType.JSON.mediaType())
             );
         }
         return Optional.empty();

diff --git a/src/main/java/org/opensearch/security/securityconf/impl/WhitelistingSettings.java b/src/main/java/org/opensearch/security/securityconf/impl/WhitelistingSettings.java
@@ -17,6 +17,8 @@
 import java.util.Optional;
 
 import org.apache.http.HttpStatus;
+
+import org.opensearch.common.xcontent.XContentType;
 import org.opensearch.security.filter.SecurityRequest;
 import org.opensearch.security.filter.SecurityResponse;
 
@@ -110,7 +112,7 @@ public Optional<SecurityResponse> checkRequestIsAllowed(final SecurityRequest re
         // if whitelisting is enabled but the request is not whitelisted, then return false, otherwise true.
         if (this.enabled && !requestIsWhitelisted(request)) {
             return Optional.of(
-                new SecurityResponse(HttpStatus.SC_FORBIDDEN, SecurityResponse.CONTENT_TYPE_APP_JSON, generateFailureMessage(request))
+                new SecurityResponse(HttpStatus.SC_FORBIDDEN, null, generateFailureMessage(request), XContentType.JSON.mediaType())
             );
         }
         return Optional.empty();

diff --git a/src/test/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticatorTest.java b/src/test/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticatorTest.java
@@ -855,7 +855,7 @@ private AuthenticateHeaders getAutenticateHeaders(HTTPSamlAuthenticator samlAuth
         RestRequest restRequest = new FakeRestRequest(ImmutableMap.of(), new HashMap<String, String>());
         SecurityResponse response = sendToAuthenticator(samlAuthenticator, restRequest).orElseThrow();
 
-        String wwwAuthenticateHeader = response.getHeaders().get("WWW-Authenticate");
+        String wwwAuthenticateHeader = response.getHeaders().get("WWW-Authenticate").get(0);
 
         Assert.assertNotNull(wwwAuthenticateHeader);