Refactor to optional in progress

Signed-off-by: Peter Nied <[email protected]>
peternied · Oct 4, 2023 · 2125f9f · 2125f9f
1 parent 613acab
commit 2125f9f
Show file tree

Hide file tree

Showing 10 changed files with 69 additions and 29 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
@@ -16,6 +16,7 @@
 import java.security.PrivilegedAction;
 import java.util.Collection;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Map.Entry;
 import java.util.regex.Pattern;
 
@@ -41,6 +42,7 @@
 import org.opensearch.security.auth.HTTPAuthenticator;
 import org.opensearch.security.filter.SecurityRequest;
 import org.opensearch.security.filter.SecurityRequestChannel;
+import org.opensearch.security.filter.SecurityResponse;
 import org.opensearch.security.user.AuthCredentials;
 
 public abstract class AbstractHTTPJwtAuthenticator implements HTTPAuthenticator {
@@ -238,12 +240,12 @@ public String[] extractRoles(JwtClaims claims) {
     protected abstract KeyProvider initKeyProvider(Settings settings, Path configPath) throws Exception;
 
     @Override
-    public boolean reRequestAuthentication(final SecurityRequestChannel request, AuthCredentials authCredentials) {
-        return request.completeWithResponse(
+    public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest request, AuthCredentials authCredentials) {
+        return Optional.of(new SecurityResponse(
             HttpStatus.SC_UNAUTHORIZED,
             Map.of("WWW-Authenticate", "Bearer realm=\"OpenSearch Security\""),
             ""
-        );
+        ));
     }
 
     public String getRequiredAudience() {

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
@@ -16,6 +16,7 @@
 import java.security.PrivilegedAction;
 import java.util.Collection;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Map.Entry;
 import java.util.regex.Pattern;
 
@@ -35,6 +36,7 @@
 import org.opensearch.security.auth.HTTPAuthenticator;
 import org.opensearch.security.filter.SecurityRequest;
 import org.opensearch.security.filter.SecurityRequestChannel;
+import org.opensearch.security.filter.SecurityResponse;
 import org.opensearch.security.user.AuthCredentials;
 import org.opensearch.security.util.KeyUtils;
 
@@ -172,12 +174,12 @@ private AuthCredentials extractCredentials0(final SecurityRequest request) {
     }
 
     @Override
-    public boolean reRequestAuthentication(final SecurityRequestChannel channel, AuthCredentials creds) {
-        return channel.completeWithResponse(
+    public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest channel, AuthCredentials creds) {
+        return Optional.of(new SecurityResponse(
             HttpStatus.SC_UNAUTHORIZED,
             Map.of("WWW-Authenticate", "Bearer realm=\"OpenSearch Security\""),
             ""
-        );
+        ));
     }
 
     @Override

diff --git a/src/main/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticator.java b/src/main/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticator.java
@@ -19,6 +19,7 @@
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
 import java.util.Map;
+import java.util.Optional;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -64,6 +65,7 @@
 import org.opensearch.security.filter.SecurityRequest;
 import org.opensearch.security.filter.SecurityRequestChannel;
 import org.opensearch.security.filter.SecurityRequetChannelUnsupported;
+import org.opensearch.security.filter.SecurityResponse;
 import org.opensearch.security.filter.OpenSearchRequestChannel;
 import org.opensearch.security.support.ConfigConstants;
 import org.opensearch.security.support.PemKeyReader;
@@ -176,7 +178,7 @@ public String getType() {
     }
 
     @Override
-    public boolean reRequestAuthentication(final SecurityRequestChannel request, final AuthCredentials authCredentials) {
+    public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest request, final AuthCredentials authCredentials) {
         try {
             Matcher matcher = PATTERN_PATH_PREFIX.matcher(request.path());
             final String suffix = matcher.matches() ? matcher.group(2) : null;
@@ -192,20 +194,20 @@ public boolean reRequestAuthentication(final SecurityRequestChannel request, fin
                     if (this.authTokenProcessorHandler.handle(restRequest, channel)) {
                         // The ACS response was accepted
                         securityRequestChannel.markCompleted();
-                        return true;
+                        return Optional.empty();
                     }
                 }
             }
 
             final Saml2Settings saml2Settings = this.saml2SettingsProvider.getCached();
-            return request.completeWithResponse(
+            return Optional.of(new SecurityResponse(
                 HttpStatus.SC_UNAUTHORIZED,
                 Map.of("WWW-Authenticate", getWwwAuthenticateHeader(saml2Settings)),
                 ""
-            );
+            ));
         } catch (Exception e) {
             log.error("Error in reRequestAuthentication()", e);
-            return false;
+            return Optional.empty();
         }
     }
 

diff --git a/src/main/java/org/opensearch/security/auth/HTTPAuthenticator.java b/src/main/java/org/opensearch/security/auth/HTTPAuthenticator.java
@@ -26,11 +26,14 @@
 
 package org.opensearch.security.auth;
 
+import java.util.Optional;
+
 import org.opensearch.OpenSearchSecurityException;
 import org.opensearch.common.util.concurrent.ThreadContext;
 import org.opensearch.rest.RestRequest;
 import org.opensearch.security.filter.SecurityRequest;
 import org.opensearch.security.filter.SecurityRequestChannel;
+import org.opensearch.security.filter.SecurityResponse;
 import org.opensearch.security.user.AuthCredentials;
 
 /**
@@ -76,10 +79,9 @@ public interface HTTPAuthenticator {
      * If the custom HTTP authenticator does support re-request authentication or supports authentication flows with multiple roundtrips
      * then the response will be returned which can then be sent via response channel.
      *
-     * @param channel The channel to sent back the response
+     * @param request The request to reauthenticate or not
      * @param credentials The credentials from the prior authentication attempt
-     * @return null if re-request is not supported/necessary, response object otherwise.
-     * If an object is returned {@code channel.sendResponse()} must be called so that the request completes.
+     * @return Optional response if is not supported/necessary, response object otherwise.
      */
-    boolean reRequestAuthentication(final SecurityRequestChannel channel, AuthCredentials credentials);
+    Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest request, AuthCredentials credentials);
 }
diff --git a/src/main/java/org/opensearch/security/filter/SecurityRequestChannel.java b/src/main/java/org/opensearch/security/filter/SecurityRequestChannel.java
@@ -1,7 +1,5 @@
 package org.opensearch.security.filter;
 
-import java.util.Map;
-
 /**
  * When a request is recieved by the security plugin this governs getting information about the request as well as a way to complet
  */
@@ -13,5 +11,5 @@ public interface SecurityRequestChannel extends SecurityRequest {
     public boolean hasCompleted();
 
     /** Use this channel to send a response */
-    public boolean completeWithResponse(final int statusCode, final Map<String, String> headers, final String body);
+    public boolean completeWithResponse(final SecurityResponse response);
 }
diff --git a/src/main/java/org/opensearch/security/filter/SecurityResponse.java b/src/main/java/org/opensearch/security/filter/SecurityResponse.java
@@ -0,0 +1,28 @@
+package org.opensearch.security.filter;
+
+import java.util.Map;
+
+public class SecurityResponse {
+    private final int status;
+    private final Map<String, String> headers;
+    private final String body;
+
+    public SecurityResponse(final int status, final Map<String, String> headers, final String body) {
+        this.status = status;
+        this.headers = headers;
+        this.body = body;
+    }
+
+    public int getStatus() {
+        return status;
+    }
+
+    public Map<String, String> getHeaders() {
+        return headers;
+    }
+
+    public String getBody() {
+        return body;
+    }
+
+}
diff --git a/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java b/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java
@@ -204,7 +204,7 @@ private void authorizeRequest(RestHandler original, SecurityRequestChannel reque
                 }
                 log.debug(err);
                 System.out.println("@206 - authorizeRequest 401");
-                request.completeWithResponse(HttpStatus.SC_UNAUTHORIZED, null, err);
+                request.completeWithResponse(new SecurityResponse(HttpStatus.SC_UNAUTHORIZED, null, err));
                 return;
             }
         }
@@ -218,7 +218,7 @@ public void checkAndAuthenticateRequest(SecurityRequestChannel requestChannel) t
             log.error(exception.toString());
             auditLog.logBadHeaders(requestChannel);
             System.out.println("@220 - checkAndAuthenticateRequest 403");
-            requestChannel.completeWithResponse(HttpStatus.SC_FORBIDDEN, null, exception.toString());
+            requestChannel.completeWithResponse(new SecurityResponse(HttpStatus.SC_FORBIDDEN, null, exception.toString()));
             return;
         }
 
@@ -227,7 +227,7 @@ public void checkAndAuthenticateRequest(SecurityRequestChannel requestChannel) t
             log.error(exception.toString());
             auditLog.logBadHeaders(requestChannel);
             System.out.println("@229 - checkAndAuthenticateRequest 403");
-            requestChannel.completeWithResponse(HttpStatus.SC_FORBIDDEN, null, exception.toString());
+            requestChannel.completeWithResponse(new SecurityResponse(HttpStatus.SC_FORBIDDEN, null, exception.toString()));
             return;
         }
 
@@ -248,7 +248,7 @@ public void checkAndAuthenticateRequest(SecurityRequestChannel requestChannel) t
             log.error("No ssl info", e);
             auditLog.logSSLException(requestChannel, e);
             System.out.println("@250 - authorizeRequest 403");
-            requestChannel.completeWithResponse(HttpStatus.SC_FORBIDDEN, null, null);
+            requestChannel.completeWithResponse(new SecurityResponse(HttpStatus.SC_FORBIDDEN, null, null));
             return;
         }
 

diff --git a/src/main/java/org/opensearch/security/http/HTTPBasicAuthenticator.java b/src/main/java/org/opensearch/security/http/HTTPBasicAuthenticator.java
@@ -28,6 +28,7 @@
 
 import java.nio.file.Path;
 import java.util.Map;
+import java.util.Optional;
 
 import org.apache.http.HttpStatus;
 import org.apache.logging.log4j.LogManager;
@@ -38,6 +39,7 @@
 import org.opensearch.security.auth.HTTPAuthenticator;
 import org.opensearch.security.filter.SecurityRequest;
 import org.opensearch.security.filter.SecurityRequestChannel;
+import org.opensearch.security.filter.SecurityResponse;
 import org.opensearch.security.support.HTTPHelper;
 import org.opensearch.security.user.AuthCredentials;
 
@@ -65,12 +67,12 @@ public AuthCredentials extractCredentials(final SecurityRequest request, final T
     }
 
     @Override
-    public boolean reRequestAuthentication(final SecurityRequestChannel request, AuthCredentials creds) {
-        return request.completeWithResponse(
+    public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest request, AuthCredentials creds) {
+        return Optional.of(new SecurityResponse(
             HttpStatus.SC_UNAUTHORIZED,
             Map.of("WWW-Authenticate", "Bearer realm=\"OpenSearch Security\""),
             ""
-        );
+        ));
     }
 
     @Override

diff --git a/src/main/java/org/opensearch/security/http/HTTPClientCertAuthenticator.java b/src/main/java/org/opensearch/security/http/HTTPClientCertAuthenticator.java
@@ -30,6 +30,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 
 import javax.naming.InvalidNameException;
 import javax.naming.ldap.LdapName;
@@ -44,6 +45,7 @@
 import org.opensearch.security.auth.HTTPAuthenticator;
 import org.opensearch.security.filter.SecurityRequest;
 import org.opensearch.security.filter.SecurityRequestChannel;
+import org.opensearch.security.filter.SecurityResponse;
 import org.opensearch.security.support.ConfigConstants;
 import org.opensearch.security.user.AuthCredentials;
 
@@ -98,8 +100,8 @@ public AuthCredentials extractCredentials(final SecurityRequest request, final T
     }
 
     @Override
-    public boolean reRequestAuthentication(final SecurityRequestChannel response, AuthCredentials creds) {
-        return false;
+    public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest response, AuthCredentials creds) {
+        return Optional.empty();
     }
 
     @Override

diff --git a/src/main/java/org/opensearch/security/http/HTTPProxyAuthenticator.java b/src/main/java/org/opensearch/security/http/HTTPProxyAuthenticator.java
@@ -27,6 +27,7 @@
 package org.opensearch.security.http;
 
 import java.nio.file.Path;
+import java.util.Optional;
 import java.util.regex.Pattern;
 
 import com.google.common.base.Predicates;
@@ -40,6 +41,7 @@
 import org.opensearch.security.auth.HTTPAuthenticator;
 import org.opensearch.security.filter.SecurityRequest;
 import org.opensearch.security.filter.SecurityRequestChannel;
+import org.opensearch.security.filter.SecurityResponse;
 import org.opensearch.security.support.ConfigConstants;
 import org.opensearch.security.user.AuthCredentials;
 
@@ -89,8 +91,8 @@ public AuthCredentials extractCredentials(final SecurityRequest request, final T
     }
 
     @Override
-    public boolean reRequestAuthentication(final SecurityRequestChannel response, AuthCredentials creds) {
-        return false;
+    public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest response, AuthCredentials creds) {
+        return Optional.empty();
     }
 
     @Override