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.

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

Upgrade to pac4j 6.x, require Java 17 or newer, and migrate to EE 9 #446

Merged
merged 1 commit into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion Jenkinsfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env groovy

buildPlugin(useContainerAgent: true, configurations: [
buildPlugin(useContainerAgent: false, configurations: [
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To run LiveTest on CI builds.

[platform: 'linux', jdk: 21],
[platform: 'windows', jdk: 17],
])
92 changes: 31 additions & 61 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,12 @@ under the License.
<artifactId>commons-lang3-api</artifactId>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>apache-httpcomponents-client-4-api</artifactId>
<!-- for Pac4J 6+
<groupId>io.jenkins.plugins</groupId>
<artifactId>apache-httpcomponents-client-5-api</artifactId>
-->
<groupId>io.jenkins.plugins</groupId>
<artifactId>commons-text-api</artifactId>
</dependency>
<dependency>
<groupId>io.jenkins.plugins</groupId>
<artifactId>apache-httpcomponents-client-5-api</artifactId>
</dependency>
<dependency>
<groupId>io.jenkins.plugins</groupId>
Expand All @@ -108,13 +108,9 @@ under the License.
<dependency>
<groupId>org.pac4j</groupId>
<artifactId>pac4j-saml</artifactId>
<version>5.7.2</version>
<version>6.0.6</version>
<optional>false</optional>
<exclusions>
<exclusion>
<groupId>org.apache.httpcomponents</groupId>
<artifactId>httpclient</artifactId>
</exclusion>
<exclusion>
<groupId>org.springframework</groupId>
<artifactId>spring-beans</artifactId>
Expand Down Expand Up @@ -145,52 +141,20 @@ under the License.
</exclusion>
<exclusion>
<groupId>org.bouncycastle</groupId>
<artifactId>bcpkix-jdk15on</artifactId>
</exclusion>
<exclusion>
<groupId>org.bouncycastle</groupId>
<artifactId>bcprov-jdk15on</artifactId>
<artifactId>bcpkix-jdk18on</artifactId>
</exclusion>
<exclusion>
<groupId>org.bouncycastle</groupId>
<artifactId>bcprov-jdk18on</artifactId>
</exclusion>
<exclusion>
<groupId>org.bouncycastle</groupId>
<artifactId>bcutil-jdk15on</artifactId>
</exclusion>
<exclusion>
<groupId>org.dom4j</groupId>
<artifactId>dom4j</artifactId>
<artifactId>bcutil-jdk18on</artifactId>
</exclusion>
<exclusion>
<groupId>commons-codec</groupId>
<artifactId>commons-codec</artifactId>
</exclusion>
<exclusion>
<groupId>antlr</groupId>
<artifactId>antlr</artifactId>
</exclusion>
<exclusion>
<groupId>org.hibernate</groupId>
<artifactId>hibernate-core</artifactId>
</exclusion>
<exclusion>
<groupId>org.hibernate.javax.persistence</groupId>
<artifactId>hibernate-jpa-2.1-api</artifactId>
</exclusion>
<exclusion>
<groupId>org.hibernate</groupId>
<artifactId>hibernate-entitymanager</artifactId>
</exclusion>
<exclusion>
<groupId>org.hibernate</groupId>
<artifactId>hibernate-commons-annotations</artifactId>
</exclusion>
<exclusion>
<groupId>commons-collections</groupId>
<artifactId>commons-collections</artifactId>
</exclusion>
<exclusion>
<groupId>commons-io</groupId>
<artifactId>commons-io</artifactId>
Expand All @@ -200,29 +164,21 @@ under the License.
<artifactId>commons-codec</artifactId>
</exclusion>
<exclusion>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</exclusion>
<exclusion>
<groupId>xalan</groupId>
<artifactId>xalan</artifactId>
<groupId>commons-text</groupId>
<artifactId>commons-text</artifactId>
</exclusion>
<exclusion>
<groupId>jakarta.activation</groupId>
<artifactId>jakarta.activation-api</artifactId>
<groupId>com.google.code.findbugs</groupId>
<artifactId>findbugs-annotations</artifactId>
</exclusion>
<exclusion>
<groupId>javax.annotation</groupId>
<artifactId>javax.annotation-api</artifactId>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</exclusion>
<exclusion>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
</exclusion>
<exclusion>
<groupId>com.fasterxml.woodstox</groupId>
<artifactId>woodstox-core</artifactId>
</exclusion>
<exclusion>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
Expand All @@ -235,6 +191,14 @@ under the License.
<groupId>org.apache.httpcomponents.client5</groupId>
<artifactId>httpclient5</artifactId>
</exclusion>
<exclusion>
<groupId>org.apache.httpcomponents.client5</groupId>
<artifactId>httpclient5-cache</artifactId>
</exclusion>
<exclusion>
<groupId>org.apache.httpcomponents.core5</groupId>
<artifactId>httpcore5</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
Expand All @@ -243,8 +207,14 @@ under the License.
</dependency>
<dependency>
<groupId>org.pac4j</groupId>
<artifactId>pac4j-javaee</artifactId>
<version>5.7.2</version>
<artifactId>pac4j-jakartaee</artifactId>
<version>6.0.6</version>
<exclusions>
<exclusion>
<groupId>com.google.code.findbugs</groupId>
<artifactId>findbugs-annotations</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
Expand Down
19 changes: 13 additions & 6 deletions src/main/java/org/jenkinsci/plugins/saml/OpenSAMLWrapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,18 @@
import java.util.Arrays;
import java.util.logging.Logger;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;
import org.kohsuke.stapler.StaplerRequest2;
import org.kohsuke.stapler.StaplerResponse2;
import org.opensaml.core.config.InitializationException;
import org.opensaml.core.config.InitializationService;
import org.pac4j.core.context.session.SessionStore;
import org.pac4j.core.util.generator.RandomValueGenerator;
import org.pac4j.jee.context.JEEContext;
import org.pac4j.core.context.WebContext;
import org.pac4j.core.exception.TechnicalException;
import org.pac4j.core.http.callback.NoParameterCallbackUrlResolver;
import org.pac4j.jee.context.JEEFrameworkParameters;
import org.pac4j.jee.context.session.JEESessionStoreFactory;
import org.pac4j.saml.client.SAML2Client;
import org.pac4j.saml.config.SAML2Configuration;
import static java.util.logging.Level.FINE;
Expand All @@ -48,8 +51,8 @@ public abstract class OpenSAMLWrapper<T> {
private static final BundleKeyStore KS = new BundleKeyStore();

protected SamlPluginConfig samlPluginConfig;
protected StaplerRequest request;
protected StaplerResponse response;
protected StaplerRequest2 request;
protected StaplerResponse2 response;

/**
* Initialize the OpenSaml services and run the process defined on the abstract method process().
Expand Down Expand Up @@ -91,6 +94,10 @@ protected WebContext createWebContext() {
return new JEEContext(request, response);
}

protected SessionStore createSessionStore() {
return JEESessionStoreFactory.INSTANCE.newSessionStore(new JEEFrameworkParameters(request, response));
}

/**
* @return a SAML2Client object to interact with the IdP service.
*/
Expand All @@ -112,7 +119,7 @@ protected SAML2Client createSAML2Client() {
config.setKeystorePath(encryptionData.getKeystorePath());
config.setKeystorePassword(encryptionData.getKeystorePasswordPlainText());
config.setPrivateKeyPassword(encryptionData.getPrivateKeyPasswordPlainText());
config.setKeystoreAlias(encryptionData.getPrivateKeyAlias());
config.setKeyStoreAlias(encryptionData.getPrivateKeyAlias());
} else {
if (!KS.isValid()) {
KS.init();
Expand All @@ -123,7 +130,7 @@ protected SAML2Client createSAML2Client() {
config.setKeystorePath(KS.getKeystorePath());
config.setKeystorePassword(KS.getKsPassword());
config.setPrivateKeyPassword(KS.getKsPkPassword());
config.setKeystoreAlias(KS.getKsPkAlias());
config.setKeyStoreAlias(KS.getKsPkAlias());
}

config.setMaximumAuthenticationLifetime(samlPluginConfig.getMaximumAuthenticationLifetime());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

import java.io.IOException;
import java.util.logging.Logger;
import javax.servlet.FilterChain;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import hudson.Extension;
import hudson.security.csrf.CrumbExclusion;

Expand Down
19 changes: 12 additions & 7 deletions src/main/java/org/jenkinsci/plugins/saml/SamlProfileWrapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@
package org.jenkinsci.plugins.saml;

import java.util.logging.Logger;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;
import org.kohsuke.stapler.StaplerRequest2;
import org.kohsuke.stapler.StaplerResponse2;
import org.pac4j.core.context.CallContext;
import org.pac4j.core.context.WebContext;
import org.pac4j.jee.context.session.JEESessionStore;
import org.pac4j.core.context.session.SessionStore;
import org.pac4j.core.exception.http.HttpAction;
import org.pac4j.saml.client.SAML2Client;
import org.pac4j.saml.credentials.SAML2AuthenticationCredentials;
import org.pac4j.saml.credentials.SAML2Credentials;
import org.pac4j.saml.exceptions.SAMLException;
import org.pac4j.saml.profile.SAML2Profile;
Expand All @@ -36,7 +38,7 @@ public class SamlProfileWrapper extends OpenSAMLWrapper<SAML2Profile> {
private static final Logger LOG = Logger.getLogger(SamlProfileWrapper.class.getName());


public SamlProfileWrapper(SamlPluginConfig samlPluginConfig, StaplerRequest request, StaplerResponse response) {
public SamlProfileWrapper(SamlPluginConfig samlPluginConfig, StaplerRequest2 request, StaplerResponse2 response) {
this.request = request;
this.response = response;
this.samlPluginConfig = samlPluginConfig;
Expand All @@ -48,13 +50,16 @@ public SamlProfileWrapper(SamlPluginConfig samlPluginConfig, StaplerRequest requ
@SuppressWarnings("unused")
@Override
protected SAML2Profile process() {
SAML2Credentials credentials;
SAML2AuthenticationCredentials credentials;
SAML2Profile saml2Profile;
try {
SAML2Client client = createSAML2Client();
WebContext context = createWebContext();
credentials = (SAML2Credentials) client.getCredentials(context, JEESessionStore.INSTANCE).orElse(null);
saml2Profile = (SAML2Profile) client.getUserProfile(credentials, context, JEESessionStore.INSTANCE).orElse(null);
SessionStore sessionStore = createSessionStore();
CallContext ctx = new CallContext(context, sessionStore);
SAML2Credentials unvalidated = (SAML2Credentials) client.getCredentials(ctx).orElse(null);
credentials = (SAML2AuthenticationCredentials) client.validateCredentials(ctx, unvalidated).orElse(null);
Comment on lines +60 to +61
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the migration guide, the getCredentials method has been split into the getCredentials and validateCredentials methods.

saml2Profile = (SAML2Profile) client.getUserProfile(ctx, credentials).orElse(null);
client.destroy();
} catch (HttpAction|SAMLException e) {
//if the SAMLResponse is not valid we send the user again to the IdP
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@

package org.jenkinsci.plugins.saml;

import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;
import org.pac4j.jee.context.session.JEESessionStore;
import org.kohsuke.stapler.StaplerRequest2;
import org.kohsuke.stapler.StaplerResponse2;
import org.pac4j.core.context.CallContext;
import org.pac4j.core.context.session.SessionStore;
import org.pac4j.core.exception.http.HttpAction;
import org.pac4j.core.exception.http.RedirectionAction;
import org.pac4j.core.context.WebContext;
Expand All @@ -30,7 +31,7 @@
*/
public class SamlRedirectActionWrapper extends OpenSAMLWrapper<RedirectionAction> {

public SamlRedirectActionWrapper(SamlPluginConfig samlPluginConfig, StaplerRequest request, StaplerResponse response) {
public SamlRedirectActionWrapper(SamlPluginConfig samlPluginConfig, StaplerRequest2 request, StaplerResponse2 response) {
this.request = request;
this.response = response;
this.samlPluginConfig = samlPluginConfig;
Expand All @@ -46,7 +47,9 @@ protected RedirectionAction process() throws IllegalStateException {
try {
SAML2Client client = createSAML2Client();
WebContext context = createWebContext();
RedirectionAction redirection = client.getRedirectionAction(context, JEESessionStore.INSTANCE).orElse(null);
SessionStore sessionStore = createSessionStore();
CallContext ctx = new CallContext(context, sessionStore);
RedirectionAction redirection = client.getRedirectionAction(ctx).orElse(null);
client.destroy();
return redirection;
} catch (HttpAction e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@

package org.jenkinsci.plugins.saml;

import java.io.IOException;
import org.kohsuke.stapler.HttpResponse;
import org.kohsuke.stapler.HttpResponses;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;
import org.kohsuke.stapler.StaplerRequest2;
import org.kohsuke.stapler.StaplerResponse2;
import org.pac4j.core.exception.TechnicalException;
import org.pac4j.saml.client.SAML2Client;

Expand All @@ -30,7 +29,7 @@
*/
public class SamlSPMetadataWrapper extends OpenSAMLWrapper<HttpResponse> {

public SamlSPMetadataWrapper(SamlPluginConfig samlPluginConfig, StaplerRequest request, StaplerResponse response) {
public SamlSPMetadataWrapper(SamlPluginConfig samlPluginConfig, StaplerRequest2 request, StaplerResponse2 response) {
this.request = request;
this.response = response;
this.samlPluginConfig = samlPluginConfig;
Expand Down
Loading
Loading