Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WIP] KNOX-2784 - Upgrade pac4j to 5.x.x #616

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[WIP] KNOX-2784 - Upgrade pac4j to 5.x.x #616

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion gateway-provider-security-pac4j/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,12 @@
<dependency>
<groupId>org.pac4j</groupId>
<artifactId>pac4j-cas</artifactId>
<exclusions>
<exclusion>
<groupId>com.sun.istack</groupId>
<artifactId>istack-commons-runtime</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.pac4j</groupId>
Expand Down Expand Up @@ -119,7 +125,7 @@
</dependency>
<dependency>
<groupId>org.pac4j</groupId>
<artifactId>pac4j-saml-opensamlv3</artifactId>
<artifactId>pac4j-saml</artifactId>
<exclusions>
<exclusion>
<groupId>ch.qos.logback</groupId>
Expand All @@ -143,6 +149,15 @@
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.pac4j</groupId>
<artifactId>pac4j-jee</artifactId>
</dependency>
<dependency>
<groupId>net.shibboleth.utilities</groupId>
<artifactId>java-support</artifactId>
</dependency>


<!-- Upgrade pac4j-saml dependencies to avoid known CVEs -->
<dependency>
Expand Down
9 changes: 4 additions & 5 deletions ...urity-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,20 +219,19 @@ public void init( FilterConfig filterConfig ) throws ServletException {
}


callbackFilter = new CallbackFilter();
callbackFilter = new CallbackFilter(config);
callbackFilter.init(filterConfig);
callbackFilter.setConfigOnly(config);
securityFilter = new SecurityFilter();
securityFilter = new SecurityFilter(config);
securityFilter.setClients(clientName);
securityFilter.setConfigOnly(config);

final String domainSuffix = filterConfig.getInitParameter(PAC4J_COOKIE_DOMAIN_SUFFIX_PARAM);
final String sessionStoreVar = filterConfig.getInitParameter(PAC4J_SESSION_STORE);

SessionStore sessionStore;

if(!StringUtils.isBlank(sessionStoreVar) && JEESessionStore.class.getName().contains(sessionStoreVar) ) {
sessionStore = new JEESessionStore();
/* NOTE: this is a final variable, and will be used by all requests in Knox */
sessionStore = JEESessionStore.INSTANCE;
} else {
sessionStore = new KnoxSessionStore(cryptoService, clusterName, domainSuffix, sessionStoreConfigs);
}
Expand Down
14 changes: 7 additions & 7 deletions ...curity-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jIdentityAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@
import org.apache.logging.log4j.Logger;
import org.pac4j.core.config.Config;
import org.pac4j.core.context.JEEContext;
import org.pac4j.core.profile.CommonProfile;
import org.pac4j.core.profile.ProfileManager;
import org.pac4j.core.profile.UserProfile;

import javax.security.auth.Subject;
import javax.servlet.Filter;
Expand Down Expand Up @@ -84,14 +84,14 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo

final HttpServletRequest request = (HttpServletRequest) servletRequest;
final HttpServletResponse response = (HttpServletResponse) servletResponse;
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);
final JEEContext context = new JEEContext(request, response);
/* see https://www.pac4j.org/blog/what_s_new_in_pac4j_v5.html#4-session-management */
final ProfileManager manager = new ProfileManager(context, ((Config)request.getAttribute(PAC4J_CONFIG)).getSessionStore());
final Optional<UserProfile> optional = manager.getProfile();
if (optional.isPresent()) {
CommonProfile profile = optional.get();
UserProfile profile = optional.get();
logger.debug("User authenticated as: {}", profile);
manager.remove(true);
manager.removeProfiles();
String id = null;
if (idAttribute != null) {
Object attribute = profile.getAttribute(idAttribute);
Expand Down
44 changes: 27 additions & 17 deletions ...-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/session/KnoxSessionStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,16 @@
import org.apache.knox.gateway.util.Urls;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.pac4j.core.context.ContextHelper;
import org.pac4j.core.context.WebContextHelper;
import org.pac4j.core.context.Cookie;
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.pac4j.core.util.serializer.JavaSerializer;
import org.pac4j.core.util.serializer.Serializer;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
Expand Down Expand Up @@ -63,16 +64,14 @@
*
* @since 0.8.0
*/
public class KnoxSessionStore<C extends WebContext> implements SessionStore<C> {
public class KnoxSessionStore implements SessionStore {

private static final Logger logger = LogManager.getLogger(KnoxSessionStore.class);

public static final String PAC4J_PASSWORD = "pac4j.password";

public static final String PAC4J_SESSION_PREFIX = "pac4j.session.";

private final JavaSerializationHelper javaSerializationHelper;

private final CryptoService cryptoService;

private final String clusterName;
Expand All @@ -81,6 +80,8 @@ public class KnoxSessionStore<C extends WebContext> implements SessionStore<C> {

final Map<String, String> sessionStoreConfigs;

private Serializer serializer;

public KnoxSessionStore(final CryptoService cryptoService, final String clusterName, final String domainSuffix) {
this(cryptoService, clusterName, domainSuffix, new HashMap());
}
Expand All @@ -89,17 +90,11 @@ public KnoxSessionStore(final CryptoService cryptoService,
final String clusterName,
final String domainSuffix,
final Map<String, String> sessionStoreConfigs) {
javaSerializationHelper = new JavaSerializationHelper();
this.cryptoService = cryptoService;
this.clusterName = clusterName;
this.domainSuffix = domainSuffix;
this.sessionStoreConfigs = sessionStoreConfigs;
}


@Override
public String getOrCreateSessionId(WebContext context) {
return null;
this.serializer = new JavaSerializer();
}

private Serializable uncompressDecryptBase64(final String v) {
Expand All @@ -113,7 +108,7 @@ private Serializable uncompressDecryptBase64(final String v) {
result.salt);
if (clear != null) {
try {
return javaSerializationHelper.deserializeFromBytes(unCompress(clear));
return (Serializable)serializer.deserializeFromBytes(unCompress(clear));
} catch (IOException e) {
throw new TechnicalException(e);
}
Expand All @@ -122,9 +117,24 @@ private Serializable uncompressDecryptBase64(final String v) {
return null;
}

/**
* Get or create the session identifier and initialize the session with it if
* necessary.
*
* @param context the web context
* @param createSession
* @return the optional session identifier
*/
@Override
public Optional<String> getSessionId(WebContext context,
boolean createSession) {
/* return session id from request */
return Optional.of("");
}

@Override
public Optional<Object> get(WebContext context, String key) {
final Cookie cookie = ContextHelper.getCookie(context, PAC4J_SESSION_PREFIX + key);
final Cookie cookie = WebContextHelper.getCookie(context, PAC4J_SESSION_PREFIX + key);
Object value = null;
if (cookie != null) {
value = uncompressDecryptBase64(cookie.getValue());
Expand All @@ -138,7 +148,7 @@ private String compressEncryptBase64(final Object o) {
|| (o instanceof Map<?,?> && ((Map<?,?>)o).isEmpty())) {
return null;
} else {
byte[] bytes = javaSerializationHelper.serializeToBytes((Serializable) o);
byte[] bytes = serializer.serializeToBytes(o);

/* compress the data */
try {
Expand Down Expand Up @@ -184,7 +194,7 @@ public void set(WebContext context, String key, Object value) {
throw new TechnicalException(e);
}
cookie.setHttpOnly(true);
cookie.setSecure(ContextHelper.isHttpsOrSecure(context));
cookie.setSecure(WebContextHelper.isHttpsOrSecure(context));

/*
* set the correct path for setting pac4j profile cookie.
Expand Down Expand Up @@ -281,7 +291,7 @@ private Object clearUserProfile(final Object value) {
}

@Override
public Optional<SessionStore<C>> buildFromTrackableSession(WebContext arg0, Object arg1) {
public Optional<SessionStore> buildFromTrackableSession(WebContext arg0, Object arg1) {
return Optional.empty();
}

Expand Down
11 changes: 11 additions & 0 deletions ...rovider-security-pac4j/src/test/java/org/apache/knox/gateway/pac4j/Pac4jProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import org.apache.knox.gateway.services.GatewayServices;
import org.apache.knox.gateway.services.security.AliasService;
import org.apache.knox.gateway.services.security.impl.DefaultCryptoService;
import org.easymock.Capture;
import org.easymock.CaptureType;
import org.easymock.EasyMock;
import org.junit.Test;
import org.pac4j.core.util.Pac4jConstants;
Expand All @@ -43,6 +45,8 @@
import java.util.Properties;
import java.util.stream.Collectors;

import static org.easymock.EasyMock.capture;
import static org.easymock.EasyMock.isA;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
Expand Down Expand Up @@ -105,12 +109,19 @@ public void test() throws Exception {
request.setRequestURL(KNOXSSO_SERVICE_URL + "?" + ORIGINAL_URL + "=" + HADOOP_SERVICE_URL);
request.setCookies(new Cookie[0]);
request.setServerName(LOCALHOST);

Capture<String> captureSetCookieHeaders = EasyMock.newCapture(
CaptureType.ALL);
MockHttpServletResponse response = new MockHttpServletResponse();
response.addHeader(isA(String.class), capture(captureSetCookieHeaders));

FilterChain filterChain = EasyMock.createNiceMock(FilterChain.class);
dispatcher.doFilter(request, response, filterChain);
// it should be a redirection to the idp topology
assertEquals(302, response.getStatus());
assertEquals(PAC4J_CALLBACK_URL + "?" + Pac4jDispatcherFilter.PAC4J_CALLBACK_PARAMETER + "=true&" + Pac4jConstants.DEFAULT_CLIENT_NAME_PARAMETER + "=" + CLIENT_CLASS, response.getHeaders().get("Location"));
System.out.println("Set-headers: "+captureSetCookieHeaders.getValues().size());

// we should have one cookie for the saved requested url
List<Cookie> cookies = response.getCookies();
assertEquals(3, cookies.size());
Expand Down
16 changes: 11 additions & 5 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,9 @@
<jaxws-ri.version>2.3.3</jaxws-ri.version>
<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.2</java-support.version>
<javassist.version>3.27.0-GA</javassist.version>
<jboss-logging.version>3.4.1.Final</jboss-logging.version>
<jee-pac4j.version>5.0.0</jee-pac4j.version>
<jee-pac4j.version>6.1.0</jee-pac4j.version>
<jericho-html.version>3.4</jericho-html.version>
<jersey.version>2.6</jersey.version>
<jetty.version>9.4.45.v20220203</jetty.version>
Expand All @@ -249,8 +248,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.5</opensaml.version>
<pac4j.version>4.5.2</pac4j.version>
<opensaml.version>4.2.0</opensaml.version>
<pac4j.version>5.4.3</pac4j.version>
<postgresql.version>42.3.3</postgresql.version>
<mysql.version>8.0.28</mysql.version>
<protobuf.version>3.16.1</protobuf.version>
Expand All @@ -277,6 +276,7 @@
<xml-jaxb.version>2.3.0</xml-jaxb.version>
<xml-matchers.version>0.10</xml-matchers.version>
<zookeeper.version>3.5.7</zookeeper.version>
<java-support.version>8.0.0</java-support.version>
</properties>

<profiles>
Expand Down Expand Up @@ -2198,7 +2198,7 @@
</dependency>
<dependency>
<groupId>org.pac4j</groupId>
<artifactId>pac4j-saml-opensamlv3</artifactId>
<artifactId>pac4j-saml</artifactId>
<version>${pac4j.version}</version>
<exclusions>
<exclusion>
Expand Down Expand Up @@ -2242,6 +2242,12 @@
</exclusions>
</dependency>

<dependency>
<groupId>org.pac4j</groupId>
<artifactId>pac4j-jee</artifactId>
<version>${pac4j.version}</version>
</dependency>

<!-- harmonize pac4j-saml dependencies -->
<dependency>
<groupId>org.opensaml</groupId>
Expand Down