You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@knox.apache.org by kr...@apache.org on 2020/08/03 15:58:28 UTC

[knox] branch master updated: KNOX-2337 - Upgrade pac4j to 4.0.3 and opensaml to 3.4.5 (#308)

This is an automated email from the ASF dual-hosted git repository.

krisden pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git


The following commit(s) were added to refs/heads/master by this push:
     new cfcb08e  KNOX-2337 - Upgrade pac4j to 4.0.3 and opensaml to 3.4.5 (#308)
cfcb08e is described below

commit cfcb08e062a1f25da2bfd7bddb2c8e7d544110c6
Author: Kevin Risden <ri...@users.noreply.github.com>
AuthorDate: Mon Aug 3 11:58:22 2020 -0400

    KNOX-2337 - Upgrade pac4j to 4.0.3 and opensaml to 3.4.5 (#308)
    
    Signed-off-by: Kevin Risden <kr...@apache.org>
---
 gateway-provider-security-pac4j/pom.xml            |  4 +--
 .../pac4j/filter/Pac4jDispatcherFilter.java        | 12 +++----
 .../gateway/pac4j/filter/Pac4jIdentityAdapter.java |  4 +--
 .../gateway/pac4j/session/KnoxSessionStore.java    | 29 ++++++++--------
 .../knox/gateway/pac4j/Pac4jProviderTest.java      | 39 ++++++++++++++--------
 pom.xml                                            | 12 +++----
 6 files changed, 57 insertions(+), 43 deletions(-)

diff --git a/gateway-provider-security-pac4j/pom.xml b/gateway-provider-security-pac4j/pom.xml
index 9e2f951..a8d41fd 100644
--- a/gateway-provider-security-pac4j/pom.xml
+++ b/gateway-provider-security-pac4j/pom.xml
@@ -71,7 +71,7 @@
 
         <dependency>
             <groupId>org.pac4j</groupId>
-            <artifactId>j2e-pac4j</artifactId>
+            <artifactId>jee-pac4j</artifactId>
             <exclusions>
                 <exclusion>
                     <groupId>org.pac4j</groupId>
@@ -113,7 +113,7 @@
         </dependency>
         <dependency>
             <groupId>org.pac4j</groupId>
-            <artifactId>pac4j-saml</artifactId>
+            <artifactId>pac4j-saml-opensamlv3</artifactId>
             <exclusions>
                 <exclusion>
                     <groupId>ch.qos.logback</groupId>
diff --git a/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilter.java b/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilter.java
index cd6beaf..bfea7ab 100644
--- a/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilter.java
+++ b/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilter.java
@@ -34,13 +34,13 @@ import org.pac4j.config.client.PropertiesConfigFactory;
 import org.pac4j.config.client.PropertiesConstants;
 import org.pac4j.core.client.Client;
 import org.pac4j.core.config.Config;
-import org.pac4j.core.context.session.J2ESessionStore;
+import org.pac4j.core.context.session.JEESessionStore;
 import org.pac4j.core.context.session.SessionStore;
 import org.pac4j.core.util.CommonHelper;
 import org.pac4j.http.client.indirect.IndirectBasicAuthClient;
 import org.pac4j.http.credentials.authenticator.test.SimpleTestUsernamePasswordAuthenticator;
-import org.pac4j.j2e.filter.CallbackFilter;
-import org.pac4j.j2e.filter.SecurityFilter;
+import org.pac4j.jee.filter.CallbackFilter;
+import org.pac4j.jee.filter.SecurityFilter;
 import org.pac4j.oidc.client.AzureAdClient;
 import org.pac4j.saml.client.SAML2Client;
 
@@ -204,8 +204,8 @@ public class Pac4jDispatcherFilter implements Filter {
 
     SessionStore sessionStore;
 
-    if(!StringUtils.isBlank(sessionStoreVar) && J2ESessionStore.class.getName().contains(sessionStoreVar) ) {
-      sessionStore = new J2ESessionStore();
+    if(!StringUtils.isBlank(sessionStoreVar) && JEESessionStore.class.getName().contains(sessionStoreVar) ) {
+      sessionStore = new JEESessionStore();
     } else {
       sessionStore = new KnoxSessionStore(cryptoService, clusterName, domainSuffix);
     }
@@ -253,7 +253,7 @@ public class Pac4jDispatcherFilter implements Filter {
   public void doFilter( ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) throws IOException, ServletException {
 
     final HttpServletRequest request = (HttpServletRequest) servletRequest;
-    request.setAttribute(PAC4J_CONFIG, securityFilter.getConfig());
+    request.setAttribute(PAC4J_CONFIG, securityFilter.getSharedConfig());
 
     // it's a callback from an identity provider
     if (request.getParameter(PAC4J_CALLBACK_PARAMETER) != null || (
diff --git a/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jIdentityAdapter.java b/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jIdentityAdapter.java
index f3d2c52..dc74510 100644
--- a/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jIdentityAdapter.java
+++ b/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jIdentityAdapter.java
@@ -27,7 +27,7 @@ import org.apache.knox.gateway.audit.log4j.audit.AuditConstants;
 import org.apache.knox.gateway.filter.AbstractGatewayFilter;
 import org.apache.knox.gateway.security.PrimaryPrincipal;
 import org.pac4j.core.config.Config;
-import org.pac4j.core.context.J2EContext;
+import org.pac4j.core.context.JEEContext;
 import org.pac4j.core.profile.CommonProfile;
 import org.pac4j.core.profile.ProfileManager;
 import org.slf4j.Logger;
@@ -83,7 +83,7 @@ public class Pac4jIdentityAdapter implements Filter {
 
     final HttpServletRequest request = (HttpServletRequest) servletRequest;
     final HttpServletResponse response = (HttpServletResponse) servletResponse;
-    final J2EContext context = new J2EContext(request, response,
+    final JEEContext context = new JEEContext(request, response,
         ((Config)request.getAttribute(PAC4J_CONFIG)).getSessionStore());
     final ProfileManager<CommonProfile> manager = new ProfileManager<>(context);
     final Optional<CommonProfile> optional = manager.get(true);
diff --git a/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/session/KnoxSessionStore.java b/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/session/KnoxSessionStore.java
index 3a7f6dd..7d2cb89 100644
--- a/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/session/KnoxSessionStore.java
+++ b/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/session/KnoxSessionStore.java
@@ -25,13 +25,13 @@ import org.apache.knox.gateway.services.security.EncryptionResult;
 import org.apache.knox.gateway.util.Urls;
 import org.pac4j.core.context.ContextHelper;
 import org.pac4j.core.context.Cookie;
-import org.pac4j.core.context.J2EContext;
-import org.pac4j.core.context.Pac4jConstants;
+import org.pac4j.core.context.JEEContext;
 import org.pac4j.core.context.WebContext;
 import org.pac4j.core.context.session.SessionStore;
 import org.pac4j.core.exception.TechnicalException;
 import org.pac4j.core.profile.CommonProfile;
 import org.pac4j.core.util.JavaSerializationHelper;
+import org.pac4j.core.util.Pac4jConstants;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -40,6 +40,7 @@ import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.Serializable;
 import java.util.Map;
+import java.util.Optional;
 import java.util.zip.GZIPInputStream;
 import java.util.zip.GZIPOutputStream;
 
@@ -49,7 +50,7 @@ import java.util.zip.GZIPOutputStream;
  *
  * @since 0.8.0
  */
-public class KnoxSessionStore implements SessionStore {
+public class KnoxSessionStore<C extends WebContext> implements SessionStore<C> {
 
     private static final Logger logger = LoggerFactory.getLogger(KnoxSessionStore.class);
 
@@ -89,7 +90,7 @@ public class KnoxSessionStore implements SessionStore {
                 result.salt);
             if (clear != null) {
                 try {
-                    return javaSerializationHelper.unserializeFromBytes(unCompress(clear));
+                    return javaSerializationHelper.deserializeFromBytes(unCompress(clear));
                 } catch (IOException e) {
                     throw new TechnicalException(e);
                 }
@@ -99,14 +100,14 @@ public class KnoxSessionStore implements SessionStore {
     }
 
     @Override
-    public Object get(WebContext context, String key) {
+    public Optional<Object> get(WebContext context, String key) {
         final Cookie cookie = ContextHelper.getCookie(context, PAC4J_SESSION_PREFIX + key);
         Object value = null;
         if (cookie != null) {
             value = uncompressDecryptBase64(cookie.getValue());
         }
         logger.debug("Get from session: {} = {}", key, value);
-        return value;
+        return Optional.ofNullable(value);
     }
 
     private String compressEncryptBase64(final Object o) {
@@ -162,7 +163,7 @@ public class KnoxSessionStore implements SessionStore {
         cookie.setHttpOnly(true);
         cookie.setSecure(ContextHelper.isHttpsOrSecure(context));
 
-        /**
+        /*
          *  set the correct path for setting pac4j profile cookie.
          *  This is because, Pac4jDispatcherFilter.PAC4J_CALLBACK_PARAMETER in the path
          *  indicates callback when ? cannot be used.
@@ -170,7 +171,7 @@ public class KnoxSessionStore implements SessionStore {
         if (context.getPath() != null && context.getPath()
             .contains(Pac4jDispatcherFilter.PAC4J_CALLBACK_PARAMETER)) {
 
-            final String[] parts = ((J2EContext) context).getRequest().getRequestURI()
+            final String[] parts = ((JEEContext) context).getNativeRequest().getRequestURI()
                 .split(
                     "websso"+ Pac4jDispatcherFilter.URL_PATH_SEPARATOR + Pac4jDispatcherFilter.PAC4J_CALLBACK_PARAMETER);
 
@@ -220,18 +221,18 @@ public class KnoxSessionStore implements SessionStore {
     private Object clearUserProfile(final Object value) {
         if(value instanceof Map<?,?>) {
             final Map<String, CommonProfile> profiles = (Map<String, CommonProfile>) value;
-            profiles.forEach((name, profile) -> profile.clearSensitiveData());
+            profiles.forEach((name, profile) -> profile.removeLoginData());
             return profiles;
         } else {
             final CommonProfile profile = (CommonProfile) value;
-            profile.clearSensitiveData();
+            profile.removeLoginData();
             return profile;
         }
     }
 
     @Override
-    public SessionStore buildFromTrackableSession(WebContext arg0, Object arg1) {
-        return null;
+    public Optional<SessionStore<C>> buildFromTrackableSession(WebContext arg0, Object arg1) {
+        return Optional.empty();
     }
 
     @Override
@@ -240,8 +241,8 @@ public class KnoxSessionStore implements SessionStore {
     }
 
     @Override
-    public Object getTrackableSession(WebContext arg0) {
-        return null;
+    public Optional getTrackableSession(WebContext arg0) {
+        return Optional.empty();
     }
 
     @Override
diff --git a/gateway-provider-security-pac4j/src/test/java/org/apache/knox/gateway/pac4j/Pac4jProviderTest.java b/gateway-provider-security-pac4j/src/test/java/org/apache/knox/gateway/pac4j/Pac4jProviderTest.java
index 7d2d260..6671554 100644
--- a/gateway-provider-security-pac4j/src/test/java/org/apache/knox/gateway/pac4j/Pac4jProviderTest.java
+++ b/gateway-provider-security-pac4j/src/test/java/org/apache/knox/gateway/pac4j/Pac4jProviderTest.java
@@ -29,7 +29,7 @@ import org.apache.knox.gateway.services.security.AliasService;
 import org.apache.knox.gateway.services.security.impl.DefaultCryptoService;
 import org.easymock.EasyMock;
 import org.junit.Test;
-import org.pac4j.core.context.Pac4jConstants;
+import org.pac4j.core.util.Pac4jConstants;
 import org.pac4j.http.client.indirect.IndirectBasicAuthClient;
 
 import javax.servlet.FilterChain;
@@ -39,6 +39,7 @@ import javax.servlet.http.Cookie;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
@@ -110,8 +111,10 @@ public class Pac4jProviderTest {
         assertEquals(PAC4J_CALLBACK_URL + "?" + Pac4jDispatcherFilter.PAC4J_CALLBACK_PARAMETER + "=true&" + Pac4jConstants.DEFAULT_CLIENT_NAME_PARAMETER + "=" + CLIENT_CLASS, response.getHeaders().get("Location"));
         // we should have one cookie for the saved requested url
         List<Cookie> cookies = response.getCookies();
-        assertEquals(1, cookies.size());
-        final Cookie requestedUrlCookie = cookies.get(0);
+        assertEquals(3, cookies.size());
+        final Cookie requestedUrlCookie = cookies.stream()
+            .filter(c -> (KnoxSessionStore.PAC4J_SESSION_PREFIX + Pac4jConstants.REQUESTED_URL).equals(c.getName()))
+            .collect(Collectors.toList()).get(0);
         assertEquals(KnoxSessionStore.PAC4J_SESSION_PREFIX + Pac4jConstants.REQUESTED_URL, requestedUrlCookie.getName());
 
         // step 2: send credentials to the callback url (callback from the identity provider)
@@ -149,8 +152,10 @@ public class Pac4jProviderTest {
         assertEquals(0, response.getStatus());
         adapter.doFilter(request, response, filterChain);
         cookies = response.getCookies();
-        assertEquals(1, cookies.size());
-        final Cookie userProfileCookie = cookies.get(0);
+        assertEquals(3, cookies.size());
+        final Cookie userProfileCookie = cookies.stream()
+            .filter(c -> (KnoxSessionStore.PAC4J_SESSION_PREFIX + Pac4jConstants.USER_PROFILES).equals(c.getName()))
+            .collect(Collectors.toList()).get(0);
         // the user profile has been cleaned
         assertEquals(KnoxSessionStore.PAC4J_SESSION_PREFIX + Pac4jConstants.USER_PROFILES, userProfileCookie.getName());
         assertNull(userProfileCookie.getValue());
@@ -209,8 +214,10 @@ public class Pac4jProviderTest {
         assertEquals(PAC4J_CALLBACK_URL + "?" + Pac4jDispatcherFilter.PAC4J_CALLBACK_PARAMETER + "=true&" + Pac4jConstants.DEFAULT_CLIENT_NAME_PARAMETER + "=" + CLIENT_CLASS, response.getHeaders().get("Location"));
         // we should have one cookie for the saved requested url
         List<Cookie> cookies = response.getCookies();
-        assertEquals(1, cookies.size());
-        final Cookie requestedUrlCookie = cookies.get(0);
+        assertEquals(3, cookies.size());
+        final Cookie requestedUrlCookie = cookies.stream()
+            .filter(c -> (KnoxSessionStore.PAC4J_SESSION_PREFIX + Pac4jConstants.REQUESTED_URL).equals(c.getName()))
+            .collect(Collectors.toList()).get(0);
         assertEquals(KnoxSessionStore.PAC4J_SESSION_PREFIX + Pac4jConstants.REQUESTED_URL, requestedUrlCookie.getName());
 
         // step 2: send credentials to the callback url (callback from the identity provider)
@@ -248,8 +255,10 @@ public class Pac4jProviderTest {
         assertEquals(0, response.getStatus());
         adapter.doFilter(request, response, filterChain);
         cookies = response.getCookies();
-        assertEquals(1, cookies.size());
-        final Cookie userProfileCookie = cookies.get(0);
+        assertEquals(3, cookies.size());
+        final Cookie userProfileCookie = cookies.stream()
+            .filter(c -> (KnoxSessionStore.PAC4J_SESSION_PREFIX + Pac4jConstants.USER_PROFILES).equals(c.getName()))
+            .collect(Collectors.toList()).get(0);
         // the user profile has been cleaned
         assertEquals(KnoxSessionStore.PAC4J_SESSION_PREFIX + Pac4jConstants.USER_PROFILES, userProfileCookie.getName());
         assertNull(userProfileCookie.getValue());
@@ -307,8 +316,10 @@ public class Pac4jProviderTest {
         assertEquals(PAC4J_CALLBACK_URL + "?" + Pac4jDispatcherFilter.PAC4J_CALLBACK_PARAMETER + "=true&" + Pac4jConstants.DEFAULT_CLIENT_NAME_PARAMETER + "=" + CLIENT_CLASS, response.getHeaders().get("Location"));
         // we should have one cookie for the saved requested url
         List<Cookie> cookies = response.getCookies();
-        assertEquals(1, cookies.size());
-        final Cookie requestedUrlCookie = cookies.get(0);
+        assertEquals(3, cookies.size());
+        final Cookie requestedUrlCookie = cookies.stream()
+            .filter(c -> (KnoxSessionStore.PAC4J_SESSION_PREFIX + Pac4jConstants.REQUESTED_URL).equals(c.getName()))
+            .collect(Collectors.toList()).get(0);
         assertEquals(KnoxSessionStore.PAC4J_SESSION_PREFIX + Pac4jConstants.REQUESTED_URL, requestedUrlCookie.getName());
 
         // step 2: send credentials to the callback url (callback from the identity provider)
@@ -346,8 +357,10 @@ public class Pac4jProviderTest {
         assertEquals(0, response.getStatus());
         adapter.doFilter(request, response, filterChain);
         cookies = response.getCookies();
-        assertEquals(1, cookies.size());
-        final Cookie userProfileCookie = cookies.get(0);
+        assertEquals(3, cookies.size());
+        final Cookie userProfileCookie = cookies.stream()
+            .filter(c -> (KnoxSessionStore.PAC4J_SESSION_PREFIX + Pac4jConstants.USER_PROFILES).equals(c.getName()))
+            .collect(Collectors.toList()).get(0);
         // the user profile has been cleaned
         assertEquals(KnoxSessionStore.PAC4J_SESSION_PREFIX + Pac4jConstants.USER_PROFILES, userProfileCookie.getName());
         assertNull(userProfileCookie.getValue());
diff --git a/pom.xml b/pom.xml
index 1ecf88a..0370b7a 100644
--- a/pom.xml
+++ b/pom.xml
@@ -201,7 +201,6 @@
         <hamcrest-json.version>0.2</hamcrest-json.version>
         <httpclient.version>4.5.12</httpclient.version>
         <httpcore.version>4.4.13</httpcore.version>
-        <j2e-pac4j.version>4.1.0</j2e-pac4j.version>
         <jackson.version>2.11.1</jackson.version>
         <jacoco-maven-plugin.version>0.8.5</jacoco-maven-plugin.version>
         <jansi.version>1.18</jansi.version>
@@ -217,6 +216,7 @@
         <jakarta.xml.bind.version>2.3.2</jakarta.xml.bind.version>
         <jakarta.activation.version>1.2.2</jakarta.activation.version>
         <java-support.version>7.5.1</java-support.version>
+        <jee-pac4j.version>5.0.0</jee-pac4j.version>
         <jericho-html.version>3.4</jericho-html.version>
         <jersey.version>2.6</jersey.version>
         <jetty.version>9.4.30.v20200611</jetty.version>
@@ -241,8 +241,8 @@
         <nimbus-jose-jwt.version>8.14.1</nimbus-jose-jwt.version>
         <nodejs.version>v12.18.2</nodejs.version>
         <okhttp.version>2.7.5</okhttp.version>
-        <opensaml.version>3.4.3</opensaml.version>
-        <pac4j.version>3.8.3</pac4j.version>
+        <opensaml.version>3.4.5</opensaml.version>
+        <pac4j.version>4.0.3</pac4j.version>
         <protobuf.version>3.12.4</protobuf.version>
         <rest-assured.version>4.3.1</rest-assured.version>
         <shiro.version>1.5.3</shiro.version>
@@ -2053,7 +2053,7 @@
             </dependency>
             <dependency>
                 <groupId>org.pac4j</groupId>
-                <artifactId>pac4j-saml</artifactId>
+                <artifactId>pac4j-saml-opensamlv3</artifactId>
                 <version>${pac4j.version}</version>
                 <exclusions>
                     <exclusion>
@@ -2087,8 +2087,8 @@
 
             <dependency>
                 <groupId>org.pac4j</groupId>
-                <artifactId>j2e-pac4j</artifactId>
-                <version>${j2e-pac4j.version}</version>
+                <artifactId>jee-pac4j</artifactId>
+                <version>${jee-pac4j.version}</version>
                 <exclusions>
                     <exclusion>
                         <groupId>org.pac4j</groupId>