KNOX-3077 - Add pac4j.cookie.max.age param (#972)

* KNOX-3077 - Add pac4j.cookie.max.age param

* KNOX-3077 - Review changes

* Fix NullPointer Exceptions and 503 inactive topology (via Phil Zampino)

gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilter.java

@@ -122,6 +122,12 @@ public class Pac4jDispatcherFilter implements Filter, SessionInvalidator {
 
   private static final String PAC4J_OIDC_TYPE = "oidc.type";
 
+  /* property for specifying pac4j cookies ttl */
+  public static final String PAC4J_COOKIE_MAX_AGE = "pac4j.cookie.max.age";
+
+  /* default value is same is KNOXSSO token ttl default */
+  private static final String PAC4J_COOKIE_MAX_AGE_DEFAULT = "-1";
+
   private CallbackFilter callbackFilter;
 
   private SecurityFilter securityFilter;
@@ -216,6 +222,8 @@ public void init( FilterConfig filterConfig ) throws ServletException {
       setSessionStoreConfig(filterConfig, PAC4J_SESSION_STORE_EXCLUDE_PERMISSIONS, PAC4J_SESSION_STORE_EXCLUDE_PERMISSIONS_DEFAULT);
       /* do we need to exclude custom attributes? */
       setSessionStoreConfig(filterConfig, PAC4J_SESSION_STORE_EXCLUDE_CUSTOM_ATTRIBUTES, PAC4J_SESSION_STORE_EXCLUDE_CUSTOM_ATTRIBUTES_DEFAULT);
+      /* add cookie expiry */
+      setSessionStoreConfig(filterConfig, PAC4J_COOKIE_MAX_AGE, PAC4J_COOKIE_MAX_AGE_DEFAULT);
       //decorating client configuration (if needed)
       PAC4J_CLIENT_CONFIGURATION_DECORATOR.decorateClients(clients, properties);
     }

gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/session/KnoxSessionStore.java

@@ -56,6 +56,7 @@
 import static org.apache.knox.gateway.pac4j.filter.Pac4jDispatcherFilter.PAC4J_SESSION_STORE_EXCLUDE_PERMISSIONS_DEFAULT;
 import static org.apache.knox.gateway.pac4j.filter.Pac4jDispatcherFilter.PAC4J_SESSION_STORE_EXCLUDE_ROLES;
 import static org.apache.knox.gateway.pac4j.filter.Pac4jDispatcherFilter.PAC4J_SESSION_STORE_EXCLUDE_ROLES_DEFAULT;
+import static org.apache.knox.gateway.pac4j.filter.Pac4jDispatcherFilter.PAC4J_COOKIE_MAX_AGE;
 
 /**
  * Specific session store where data are saved into cookies (and not in memory).
@@ -201,6 +202,11 @@ public void set(WebContext context, String key, Object value) {
             cookie.setPath(parts[0]);
 
         }
+
+        /* Set cookie max age */
+        if(sessionStoreConfigs != null && sessionStoreConfigs.containsKey(PAC4J_COOKIE_MAX_AGE)) {
+            cookie.setMaxAge(Integer.parseInt(sessionStoreConfigs.get(PAC4J_COOKIE_MAX_AGE)));
+        }
         context.addResponseCookie(cookie);
     }
 

gateway-server/src/test/java/org/apache/knox/gateway/util/KnoxCLITest.java

@@ -33,6 +33,7 @@
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.PrintStream;
+import java.lang.reflect.Field;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
@@ -41,6 +42,7 @@
 
 import org.apache.commons.io.FileUtils;
 import org.apache.hadoop.conf.Configuration;
+import org.apache.knox.gateway.GatewayServer;
 import org.apache.knox.gateway.config.impl.GatewayConfigImpl;
 import org.apache.knox.gateway.model.DescriptorConfiguration;
 import org.apache.knox.gateway.model.ProviderConfiguration;
@@ -71,6 +73,7 @@ public class KnoxCLITest {
   public void setUp() throws Exception {
     System.setOut(new PrintStream(outContent, false, StandardCharsets.UTF_8.name()));
     System.setErr(new PrintStream(errContent, false, StandardCharsets.UTF_8.name()));
+    this.setGatewayServicesToNull();
   }
 
   @Test
@@ -1337,6 +1340,12 @@ private File createDir() throws IOException {
         .createTempDir(this.getClass().getSimpleName() + "-");
   }
 
+  private void setGatewayServicesToNull() throws Exception {
+    Field gwsField = GatewayServer.class.getDeclaredField("services");
+    gwsField.setAccessible(true);
+    gwsField.set(null, null);
+  }
+
   private static final String testDescriptorContentJSON = "{\n" +
                                                           "  \"discovery-address\":\"http://localhost:8080\",\n" +
                                                           "  \"discovery-user\":\"maria_dev\",\n" +

gateway-test/src/test/java/org/apache/knox/gateway/GatewayDeployFuncTest.java

@@ -17,16 +17,16 @@
  */
 package org.apache.knox.gateway;
 
-import io.restassured.response.Response;
 import com.mycila.xmltool.XMLDoc;
 import com.mycila.xmltool.XMLTag;
+import io.restassured.response.Response;
 import org.apache.commons.io.FileUtils;
+import org.apache.http.HttpStatus;
 import org.apache.knox.gateway.config.GatewayConfig;
 import org.apache.knox.gateway.services.DefaultGatewayServices;
 import org.apache.knox.gateway.services.ServiceLifecycleException;
 import org.apache.knox.test.TestUtils;
 import org.apache.knox.test.category.ReleaseTest;
-import org.apache.http.HttpStatus;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.junit.After;
@@ -283,7 +283,7 @@ private void waitForAccess( String url, String username, String password, long s
       Response response = given()
           .auth().preemptive().basic( username, password )
           .when().get( url ).andReturn();
-      if( response.getStatusCode() == HttpStatus.SC_NOT_FOUND ) {
+      if( response.getStatusCode() == HttpStatus.SC_NOT_FOUND || response.getStatusCode() == HttpStatus.SC_SERVICE_UNAVAILABLE ) {
         Thread.sleep( sleep );
         continue;
       }

gateway-test/src/test/java/org/apache/knox/gateway/GatewayLdapPosixGroupFuncTest.java

@@ -19,6 +19,7 @@
 
 import com.mycila.xmltool.XMLDoc;
 import com.mycila.xmltool.XMLTag;
+import io.restassured.response.Response;
 import org.apache.knox.gateway.services.ServiceType;
 import org.apache.knox.gateway.services.GatewayServices;
 import org.apache.knox.gateway.services.security.AliasService;
@@ -36,7 +37,10 @@
 import static io.restassured.RestAssured.given;
 import static org.apache.knox.test.TestUtils.LOG_ENTER;
 import static org.apache.knox.test.TestUtils.LOG_EXIT;
+import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
 
 /**
  * Functional test to verify : looking up ldap groups from directory
@@ -47,6 +51,7 @@
 public class GatewayLdapPosixGroupFuncTest {
   private static final GatewayTestDriver driver = new GatewayTestDriver();
   private static final String cluster = "test-cluster";
+  private static final long sleep = 200;
 
   @BeforeClass
   public static void setupSuite() throws Exception {
@@ -65,7 +70,6 @@ public static void cleanupSuite() throws Exception {
   }
 
   public static void setupGateway() throws Exception {
-    String cluster = "test-cluster";
     GatewayTestConfig config = new GatewayTestConfig();
     XMLTag topology = createTopology();
     driver.setupGateway(config, cluster, topology, true);
@@ -169,37 +173,40 @@ private static XMLTag createTopology() {
   }
 
   @Test( timeout = TestUtils.MEDIUM_TIMEOUT )
-  public void testGroupMember() {
+  public void testGroupMember() throws InterruptedException {
     LOG_ENTER();
     String username = "sam";
     String password = "sam-password";
     String serviceUrl = driver.getClusterUrl() + "/test-service-path/test-service-resource";
-    given()
-        //.log().all()
-        .auth().preemptive().basic( username, password )
-        .then()
-        //.log().all()
-        .statusCode( HttpStatus.SC_OK )
-        .contentType( "text/plain" )
-        .body( is( "test-service-response" ) )
-        .when().get( serviceUrl );
+    Response response = waitForActiveTopology(serviceUrl, username, password);
+    assertEquals( HttpStatus.SC_OK, response.getStatusCode() );
+    assertThat( response.getContentType(), containsString( "text/plain" ) );
+    assertThat( response.getBody().asString(), is( "test-service-response" ) );
     LOG_EXIT();
   }
 
   @Test( timeout = TestUtils.MEDIUM_TIMEOUT )
-  public void testNonGroupMember() {
+  public void testNonGroupMember() throws InterruptedException {
     LOG_ENTER();
     String username = "guest";
     String password = "guest-password";
     String serviceUrl = driver.getClusterUrl() + "/test-service-path/test-service-resource";
-    given()
-        //.log().all()
-        .auth().preemptive().basic( username, password )
-        .then()
-        //.log().all()
-        .statusCode( HttpStatus.SC_FORBIDDEN )
-        .when().get( serviceUrl );
+    Response response = waitForActiveTopology(serviceUrl, username, password);
+    assertEquals( HttpStatus.SC_FORBIDDEN, response.getStatusCode() );
     LOG_EXIT();
   }
 
+  private Response waitForActiveTopology( String url, String username, String password ) throws InterruptedException {
+    while( true ) {
+      Response response = given()
+              .auth().preemptive().basic( username, password )
+              .when().get( url ).andReturn();
+      if( response.getStatusCode() == HttpStatus.SC_NOT_FOUND || response.getStatusCode() == HttpStatus.SC_SERVICE_UNAVAILABLE ) {
+        Thread.sleep( sleep );
+        continue;
+      }
+      return response;
+    }
+  }
+
 }