You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@knox.apache.org by mo...@apache.org on 2017/02/20 14:56:03 UTC

knox git commit: KNOX-869 - Support for multiple pre-auth validaors (Mohammad Kamrul Islam via Sandeep More)

Repository: knox
Updated Branches:
  refs/heads/master 9af4fe401 -> c23759ae2


KNOX-869 - Support for multiple pre-auth validaors (Mohammad Kamrul Islam via Sandeep More)


Project: http://git-wip-us.apache.org/repos/asf/knox/repo
Commit: http://git-wip-us.apache.org/repos/asf/knox/commit/c23759ae
Tree: http://git-wip-us.apache.org/repos/asf/knox/tree/c23759ae
Diff: http://git-wip-us.apache.org/repos/asf/knox/diff/c23759ae

Branch: refs/heads/master
Commit: c23759ae26b8d3adb49643044c26b563a56a3c5c
Parents: 9af4fe4
Author: Sandeep More <mo...@apache.org>
Authored: Mon Feb 20 09:55:49 2017 -0500
Committer: Sandeep More <mo...@apache.org>
Committed: Mon Feb 20 09:55:49 2017 -0500

----------------------------------------------------------------------
 .../filter/AbstractPreAuthFederationFilter.java | 23 +++------
 .../preauth/filter/PreAuthFederationFilter.java | 19 ++------
 .../gateway/preauth/filter/PreAuthService.java  | 44 +++++++++++++----
 .../HeaderPreAuthFederationFilterTest.java      | 32 ++++++++-----
 .../provider/federation/PreAuthServiceTest.java | 50 +++++++++++++++++---
 5 files changed, 107 insertions(+), 61 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/knox/blob/c23759ae/gateway-provider-security-preauth/src/main/java/org/apache/hadoop/gateway/preauth/filter/AbstractPreAuthFederationFilter.java
----------------------------------------------------------------------
diff --git a/gateway-provider-security-preauth/src/main/java/org/apache/hadoop/gateway/preauth/filter/AbstractPreAuthFederationFilter.java b/gateway-provider-security-preauth/src/main/java/org/apache/hadoop/gateway/preauth/filter/AbstractPreAuthFederationFilter.java
index b40de0a..3a435e4 100644
--- a/gateway-provider-security-preauth/src/main/java/org/apache/hadoop/gateway/preauth/filter/AbstractPreAuthFederationFilter.java
+++ b/gateway-provider-security-preauth/src/main/java/org/apache/hadoop/gateway/preauth/filter/AbstractPreAuthFederationFilter.java
@@ -21,6 +21,7 @@ import java.io.IOException;
 import java.security.Principal;
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
+import java.util.List;
 import java.util.Set;
 
 import javax.security.auth.Subject;
@@ -41,7 +42,7 @@ import org.apache.hadoop.gateway.security.PrimaryPrincipal;
  */
 public abstract class AbstractPreAuthFederationFilter implements Filter {
 
-  private PreAuthValidator validator = null;
+  private List<PreAuthValidator> validators = null;
   private FilterConfig filterConfig;
 
   /**
@@ -54,12 +55,12 @@ public abstract class AbstractPreAuthFederationFilter implements Filter {
   @Override
   public void init(FilterConfig filterConfig) throws ServletException {
     this.filterConfig = filterConfig;
-    validator = PreAuthService.getValidator(filterConfig);
+    validators = PreAuthService.getValidators(filterConfig);
   }
 
   @VisibleForTesting
-  public PreAuthValidator getValidator() {
-    return validator;
+  public List<PreAuthValidator> getValidators() {
+    return validators;
   }
 
   @Override
@@ -68,7 +69,7 @@ public abstract class AbstractPreAuthFederationFilter implements Filter {
     HttpServletRequest httpRequest = (HttpServletRequest)request;
     String principal = getPrimaryPrincipal(httpRequest);
     if (principal != null) {
-      if (isValid(httpRequest)) {
+      if (PreAuthService.validate(httpRequest, filterConfig, validators)) {
         Subject subject = new Subject();
         subject.getPrincipals().add(new PrimaryPrincipal(principal));
         addGroupPrincipals(httpRequest, subject.getPrincipals());
@@ -84,18 +85,6 @@ public abstract class AbstractPreAuthFederationFilter implements Filter {
     }
   }
 
-  /**
-   * @return
-   */
-  private boolean isValid(HttpServletRequest httpRequest) {
-    try {
-      return validator.validate(httpRequest, filterConfig);
-    } catch (PreAuthValidationException e) {
-      // TODO log exception
-      return false;
-    }
-  }
-
   @Override
   public void destroy() {
   }

http://git-wip-us.apache.org/repos/asf/knox/blob/c23759ae/gateway-provider-security-preauth/src/main/java/org/apache/hadoop/gateway/preauth/filter/PreAuthFederationFilter.java
----------------------------------------------------------------------
diff --git a/gateway-provider-security-preauth/src/main/java/org/apache/hadoop/gateway/preauth/filter/PreAuthFederationFilter.java b/gateway-provider-security-preauth/src/main/java/org/apache/hadoop/gateway/preauth/filter/PreAuthFederationFilter.java
index 63eb411..27ae803 100644
--- a/gateway-provider-security-preauth/src/main/java/org/apache/hadoop/gateway/preauth/filter/PreAuthFederationFilter.java
+++ b/gateway-provider-security-preauth/src/main/java/org/apache/hadoop/gateway/preauth/filter/PreAuthFederationFilter.java
@@ -22,6 +22,7 @@ import java.security.AccessController;
 import java.security.Principal;
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
+import java.util.List;
 
 import javax.security.auth.Subject;
 import javax.servlet.Filter;
@@ -37,7 +38,7 @@ import org.apache.hadoop.gateway.security.PrimaryPrincipal;
 
 public class PreAuthFederationFilter implements Filter {
   private static final String CUSTOM_HEADER_PARAM = "preauth.customHeader";
-  private PreAuthValidator validator = null;
+  private List<PreAuthValidator> validators = null;
   private FilterConfig filterConfig;
   private String headerName = "SM_USER";
 
@@ -48,7 +49,7 @@ public class PreAuthFederationFilter implements Filter {
       headerName = customHeader;
     }
     this.filterConfig = filterConfig;
-    validator = PreAuthService.getValidator(filterConfig);
+    validators = PreAuthService.getValidators(filterConfig);
   }
 
   @Override
@@ -56,7 +57,7 @@ public class PreAuthFederationFilter implements Filter {
                        FilterChain chain) throws IOException, ServletException {
     HttpServletRequest httpRequest = (HttpServletRequest) request;
     if (httpRequest.getHeader(headerName) != null) {
-      if (isValid(httpRequest)) {
+      if (PreAuthService.validate(httpRequest, filterConfig, validators)) {
         // TODO: continue as subject
         chain.doFilter(request, response);
       } else {
@@ -68,18 +69,6 @@ public class PreAuthFederationFilter implements Filter {
     }
   }
 
-  /**
-   * @return
-   */
-  private boolean isValid(HttpServletRequest httpRequest) {
-    try {
-      return validator.validate(httpRequest, filterConfig);
-    } catch (PreAuthValidationException e) {
-      // TODO log exception
-      return false;
-    }
-  }
-
   /* (non-Javadoc)
    * @see javax.servlet.Filter#destroy()
    */

http://git-wip-us.apache.org/repos/asf/knox/blob/c23759ae/gateway-provider-security-preauth/src/main/java/org/apache/hadoop/gateway/preauth/filter/PreAuthService.java
----------------------------------------------------------------------
diff --git a/gateway-provider-security-preauth/src/main/java/org/apache/hadoop/gateway/preauth/filter/PreAuthService.java b/gateway-provider-security-preauth/src/main/java/org/apache/hadoop/gateway/preauth/filter/PreAuthService.java
index 778ee9d..e1d9751 100644
--- a/gateway-provider-security-preauth/src/main/java/org/apache/hadoop/gateway/preauth/filter/PreAuthService.java
+++ b/gateway-provider-security-preauth/src/main/java/org/apache/hadoop/gateway/preauth/filter/PreAuthService.java
@@ -22,6 +22,10 @@ import com.google.common.base.Strings;
 
 import javax.servlet.FilterConfig;
 import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import java.util.ArrayList;
+import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Set;
 import java.util.Collections;
 import java.util.ServiceLoader;
@@ -62,19 +66,41 @@ public class PreAuthService {
    *
    * @since 0.12
    * @param filterConfig
-   * @return PreAuthValidator
+   * @return List<PreAuthValidator>
    * @throws ServletException
    */
-  public static PreAuthValidator getValidator(FilterConfig filterConfig) throws ServletException {
-    String validationMethod = filterConfig.getInitParameter(VALIDATION_METHOD_PARAM);
-    if (Strings.isNullOrEmpty(validationMethod)) {
-      validationMethod = DefaultValidator.DEFAULT_VALIDATION_METHOD_VALUE;
+  public static List<PreAuthValidator> getValidators(FilterConfig filterConfig) throws ServletException {
+    String validationMethods = filterConfig.getInitParameter(VALIDATION_METHOD_PARAM);
+    List<PreAuthValidator> vList = new ArrayList<>();
+    if (Strings.isNullOrEmpty(validationMethods)) {
+      validationMethods = DefaultValidator.DEFAULT_VALIDATION_METHOD_VALUE;
     }
-    if (validatorMap.containsKey(validationMethod)) {
-      return validatorMap.get(validationMethod);
-    } else {
-      throw new ServletException(String.format("Unable to find validator with name '%s'", validationMethod));
+    Set<String> vMethodSet = new LinkedHashSet<>();
+    Collections.addAll(vMethodSet, validationMethods.trim().split("\\s*,\\s*"));
+    for (String vName : vMethodSet) {
+      if (validatorMap.containsKey(vName)) {
+        vList.add(validatorMap.get(vName));
+      } else {
+        throw new ServletException(String.format("Unable to find validator with name '%s'", validationMethods));
+      }
     }
+    return vList;
+  }
+
+  public static boolean validate(HttpServletRequest httpRequest, FilterConfig filterConfig, List<PreAuthValidator>
+      validators) {
+    try {
+      for (PreAuthValidator validator : validators) {
+        //Any one validator fails, it will fail the request. loginal AND behavior
+        if (!validator.validate(httpRequest, filterConfig)) {
+          return false;
+        }
+      }
+    } catch (PreAuthValidationException e) {
+      // TODO log exception
+      return false;
+    }
+    return true;
   }
 
 }

http://git-wip-us.apache.org/repos/asf/knox/blob/c23759ae/gateway-provider-security-preauth/src/test/java/org/apache/hadoop/gateway/provider/federation/HeaderPreAuthFederationFilterTest.java
----------------------------------------------------------------------
diff --git a/gateway-provider-security-preauth/src/test/java/org/apache/hadoop/gateway/provider/federation/HeaderPreAuthFederationFilterTest.java b/gateway-provider-security-preauth/src/test/java/org/apache/hadoop/gateway/provider/federation/HeaderPreAuthFederationFilterTest.java
index 28d7eeb..365ec45 100644
--- a/gateway-provider-security-preauth/src/test/java/org/apache/hadoop/gateway/provider/federation/HeaderPreAuthFederationFilterTest.java
+++ b/gateway-provider-security-preauth/src/test/java/org/apache/hadoop/gateway/provider/federation/HeaderPreAuthFederationFilterTest.java
@@ -25,6 +25,8 @@ import javax.servlet.FilterConfig;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 
+import java.util.List;
+
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -38,9 +40,10 @@ public class HeaderPreAuthFederationFilterTest extends TestCase {
     when(filterConfig.getInitParameter(PreAuthService.VALIDATION_METHOD_PARAM)).thenReturn
         (DefaultValidator.DEFAULT_VALIDATION_METHOD_VALUE);
     hpaff.init(filterConfig);
-    PreAuthValidator validator = hpaff.getValidator();
-    assertEquals(validator.getName(), DefaultValidator.DEFAULT_VALIDATION_METHOD_VALUE);
-    assertTrue(validator.validate(request, filterConfig));
+    List<PreAuthValidator> validators = hpaff.getValidators();
+    assertEquals(validators.size(), 1);
+    assertEquals(validators.get(0).getName(), DefaultValidator.DEFAULT_VALIDATION_METHOD_VALUE);
+    assertTrue(PreAuthService.validate(request, filterConfig, validators));
   }
 
   @Test
@@ -53,12 +56,13 @@ public class HeaderPreAuthFederationFilterTest extends TestCase {
     when(filterConfig.getInitParameter(PreAuthService.VALIDATION_METHOD_PARAM)).thenReturn(IPValidator
         .IP_VALIDATION_METHOD_VALUE);
     hpaff.init(filterConfig);
-    PreAuthValidator validator = hpaff.getValidator();
-    assertEquals(validator.getName(), IPValidator.IP_VALIDATION_METHOD_VALUE);
-    assertTrue(validator.validate(request, filterConfig));
+    List<PreAuthValidator> validators = hpaff.getValidators();
+    assertEquals(validators.size(), 1);
+    assertEquals(validators.get(0).getName(), IPValidator.IP_VALIDATION_METHOD_VALUE);
+    assertTrue(PreAuthService.validate(request, filterConfig, validators));
     //Negative testing
     when(request.getRemoteAddr()).thenReturn("10.10.22.33");
-    assertFalse(validator.validate(request, filterConfig));
+    assertFalse(PreAuthService.validate(request, filterConfig, validators));
   }
 
   @Test
@@ -70,11 +74,12 @@ public class HeaderPreAuthFederationFilterTest extends TestCase {
         (DummyValidator.NAME);
 
     hpaff.init(filterConfig);
-    PreAuthValidator validator = hpaff.getValidator();
-    assertEquals(validator.getName(), DummyValidator.NAME);
+    List<PreAuthValidator> validators = hpaff.getValidators();
+    assertEquals(validators.size(), 1);
+    assertEquals(validators.get(0).getName(), DummyValidator.NAME);
     //Positive test
     when(request.getHeader("CUSTOM_TOKEN")).thenReturn("HelloWorld");
-    assertTrue(validator.validate(request, filterConfig));
+    assertTrue(PreAuthService.validate(request, filterConfig, validators));
 
   }
 
@@ -87,11 +92,12 @@ public class HeaderPreAuthFederationFilterTest extends TestCase {
         (DummyValidator.NAME);
 
     hpaff.init(filterConfig);
-    PreAuthValidator validator = hpaff.getValidator();
-    assertEquals(validator.getName(), DummyValidator.NAME);
+    List<PreAuthValidator> validators = hpaff.getValidators();
+    assertEquals(validators.size(), 1);
+    assertEquals(validators.get(0).getName(), DummyValidator.NAME);
 
     when(request.getHeader("CUSTOM_TOKEN")).thenReturn("NOTHelloWorld");
-    assertFalse(validator.validate(request, filterConfig));
+    assertFalse(PreAuthService.validate(request, filterConfig, validators));
 
   }
 

http://git-wip-us.apache.org/repos/asf/knox/blob/c23759ae/gateway-provider-security-preauth/src/test/java/org/apache/hadoop/gateway/provider/federation/PreAuthServiceTest.java
----------------------------------------------------------------------
diff --git a/gateway-provider-security-preauth/src/test/java/org/apache/hadoop/gateway/provider/federation/PreAuthServiceTest.java b/gateway-provider-security-preauth/src/test/java/org/apache/hadoop/gateway/provider/federation/PreAuthServiceTest.java
index 8186189..0332a6c 100644
--- a/gateway-provider-security-preauth/src/test/java/org/apache/hadoop/gateway/provider/federation/PreAuthServiceTest.java
+++ b/gateway-provider-security-preauth/src/test/java/org/apache/hadoop/gateway/provider/federation/PreAuthServiceTest.java
@@ -25,6 +25,7 @@ import javax.servlet.FilterConfig;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 
+import java.util.List;
 import java.util.Map;
 
 import static org.mockito.Mockito.mock;
@@ -50,9 +51,10 @@ public class PreAuthServiceTest extends TestCase {
     final FilterConfig filterConfig = mock(FilterConfig.class);
     when(filterConfig.getInitParameter(PreAuthService.VALIDATION_METHOD_PARAM)).thenReturn
         (DefaultValidator.DEFAULT_VALIDATION_METHOD_VALUE);
-    PreAuthValidator validator = PreAuthService.getValidator(filterConfig);
-    assertEquals(validator.getName(), DefaultValidator.DEFAULT_VALIDATION_METHOD_VALUE);
-    assertTrue(validator.validate(request, filterConfig));
+    List<PreAuthValidator> validators = PreAuthService.getValidators(filterConfig);
+    assertEquals(validators.size(), 1);
+    assertEquals(validators.get(0).getName(), DefaultValidator.DEFAULT_VALIDATION_METHOD_VALUE);
+    assertTrue(PreAuthService.validate(request, filterConfig, validators));
   }
 
   @Test
@@ -63,11 +65,45 @@ public class PreAuthServiceTest extends TestCase {
     when(filterConfig.getInitParameter(IPValidator.IP_ADDRESSES_PARAM)).thenReturn("5.4.3.2,10.1.23.42");
     when(filterConfig.getInitParameter(PreAuthService.VALIDATION_METHOD_PARAM)).thenReturn(IPValidator
         .IP_VALIDATION_METHOD_VALUE);
-    PreAuthValidator validator = PreAuthService.getValidator(filterConfig);
-    assertEquals(validator.getName(), IPValidator.IP_VALIDATION_METHOD_VALUE);
-    assertTrue(validator.validate(request, filterConfig));
+    List<PreAuthValidator> validators = PreAuthService.getValidators(filterConfig);
+    assertEquals(validators.size(), 1);
+    assertEquals(validators.get(0).getName(), IPValidator.IP_VALIDATION_METHOD_VALUE);
+    assertTrue(PreAuthService.validate(request, filterConfig, validators));
     //Negative testing
     when(request.getRemoteAddr()).thenReturn("10.10.22.33");
-    assertFalse(validator.validate(request, filterConfig));
+    assertFalse(PreAuthService.validate(request, filterConfig, validators));
+  }
+
+  @Test
+  public void testMultipleValidatorsPositive() throws ServletException, PreAuthValidationException {
+    final HttpServletRequest request = mock(HttpServletRequest.class);
+    when(request.getRemoteAddr()).thenReturn("10.1.23.42");
+    final FilterConfig filterConfig = mock(FilterConfig.class);
+    when(filterConfig.getInitParameter(IPValidator.IP_ADDRESSES_PARAM)).thenReturn("5.4.3.2,10.1.23.42");
+    when(filterConfig.getInitParameter(PreAuthService.VALIDATION_METHOD_PARAM)).thenReturn
+        (DefaultValidator.DEFAULT_VALIDATION_METHOD_VALUE + "," + IPValidator.IP_VALIDATION_METHOD_VALUE );
+    List<PreAuthValidator> validators = PreAuthService.getValidators(filterConfig);
+    assertEquals(validators.size(), 2);
+    assertEquals(validators.get(0).getName(), DefaultValidator.DEFAULT_VALIDATION_METHOD_VALUE);
+    assertEquals(validators.get(1).getName(), IPValidator.IP_VALIDATION_METHOD_VALUE);
+
+    assertTrue(PreAuthService.validate(request, filterConfig, validators));
+    //Negative testing
+    when(request.getRemoteAddr()).thenReturn("10.10.22.33");
+    assertFalse(PreAuthService.validate(request, filterConfig, validators));
+
+  }
+
+  @Test
+  public void testMultipleValidatorsNegative() throws ServletException, PreAuthValidationException {
+    final FilterConfig filterConfig = mock(FilterConfig.class);
+    when(filterConfig.getInitParameter(PreAuthService.VALIDATION_METHOD_PARAM)).thenReturn
+        (DefaultValidator.DEFAULT_VALIDATION_METHOD_VALUE + ",  NOT_EXISTED_VALIDATOR" );
+    try {
+      PreAuthService.getValidators(filterConfig);
+      fail("Should throw exception due to invalid validator");
+    } catch (Exception e) {
+      //Expected
+    }
   }
 }