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

[JENKINS-63350] update pac4j to 3.9.0 #90

Merged
merged 17 commits into from
Feb 25, 2021
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 .mvn/extensions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
<extension>
<groupId>io.jenkins.tools.incrementals</groupId>
<artifactId>git-changelist-maven-extension</artifactId>
<version>1.0-beta-7</version>
<version>1.2</version>
</extension>
</extensions>
17 changes: 9 additions & 8 deletions Jenkinsfile
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
//def buildConfiguration = buildPlugin.recommendedConfigurations()

def lts = "2.176.1"
def weekly = "2.199"
def lts = "2.249.3"
Copy link
Member

Choose a reason for hiding this comment

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

??

def buildConfiguration = [
[ platform: "linux", jdk: "8", jenkins: lts, javaLevel: "8" ],
[ platform: "windows", jdk: "8", jenkins: lts, javaLevel: "8" ],
[ platform: "linux", jdk: "11", jenkins: lts, javaLevel: "8" ],
[ platform: "windows", jdk: "11", jenkins: lts, javaLevel: "8" ],
/*
[ platform: "linux", jdk: "8", jenkins: lts ],
[ platform: "windows", jdk: "8", jenkins: lts ],
[ platform: "linux", jdk: "11", jenkins: lts ],
[ platform: "windows", jdk: "11", jenkins: lts ],
*/
// Also build on recent weekly
// [ platform: "linux", jdk: "11", jenkins: weekly, javaLevel: "8" ],
// [ platform: "windows", jdk: "11", jenkins: weekly, javaLevel: "8" ]
[ platform: "linux", jdk: "11" ],
[ platform: "windows", jdk: "11" ]
]

buildPlugin(configurations: buildConfiguration)
111 changes: 87 additions & 24 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ under the License.
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>3.55</version>
<version>4.3</version>
</parent>

<artifactId>saml</artifactId>
Expand All @@ -42,9 +42,9 @@ under the License.
</scm>

<properties>
<revision>1.1.8</revision>
<revision>2.0.0</revision>
<changelist>-SNAPSHOT</changelist>
<jenkins.version>2.176.1</jenkins.version>
<jenkins.version>2.266</jenkins.version>
<java.level>8</java.level>
<jcasc.version>1.35</jcasc.version>
</properties>
Expand Down Expand Up @@ -90,8 +90,26 @@ under the License.
<dependency>
<groupId>org.pac4j</groupId>
<artifactId>pac4j-saml</artifactId>
<version>1.9.9</version>
<!-- versions 4.x.x require JDK 11 -->
Copy link
Member

Choose a reason for hiding this comment

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

Then set java.level to 11 I guess? Not sure of status of jenkinsci/plugin-pom#133.

Copy link
Author

Choose a reason for hiding this comment

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

I think is fine, 3.9.0 was released a few days ago

<version>3.9.0</version>
<optional>false</optional>
<exclusions>
<exclusion>
<groupId>org.springframework</groupId>
<artifactId>spring-beans</artifactId>
Copy link
Member

Choose a reason for hiding this comment

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

You may need to exclude more. Pay attention to what gets bundled in the *.hpi (also printed to build log).

Copy link
Author

Choose a reason for hiding this comment

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

yep, there is a bunch of transitive dependencies that can be removed

Copy link
Author

Choose a reason for hiding this comment

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

is there any way to acknowledge(remove the warnings) the dependencies? there are some warnings that come from the hpi plugin, but those dependencies are needed and I do not want to add all those dependencies manually to the plugin pom.

Copy link
Member

Choose a reason for hiding this comment

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

is there any way to acknowledge(remove the warnings)

Yes but do not do that.

there are some warnings that come from the hpi plugin, but those dependencies are needed

You can exclude dependencies you know you will not use, or you know you can use in the version supplied by core. Or you can switch to pluginFirstClassLoader and forget about accurate testing with JenkinsRule (must do it manually). Think twice, actually several times, before doing anything out of the ordinary. https://www.jenkins.io/doc/developer/plugin-development/dependencies-and-class-loading/

Copy link
Author

Choose a reason for hiding this comment

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

I have excluded all that are not needed or are already in the Jenkins Core. so I guess that either I add these libraries to the pom or I live with these warnings


[WARNING] Bundling transitive dependency jcommander-1.48.jar (via pac4j-saml)
--
[WARNING] Bundling transitive dependency woodstox-core-5.0.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency istack-commons-runtime-3.0.8.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency FastInfoset-1.2.16.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency metrics-core-3.1.2.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency jakarta.xml.bind-api-2.3.2.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency javax.json-api-1.0.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency joda-time-2.9.9.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency xmlsectool-2.0.0.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency java-support-7.5.0.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency spymemcached-2.12.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency httpclient-4.5.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency httpcore-4.4.8.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency xmlsec-2.1.4.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency velocity-1.7.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency stax2-api-3.1.4.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency cryptacular-1.2.4.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency jaxb-runtime-2.3.2.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency txw2-2.3.2.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency javax.json-1.0.4.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency stax-ex-1.8.1.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency ldaptive-1.0.13.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-core-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-messaging-api-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-messaging-impl-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-profile-api-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-profile-impl-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-saml-api-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-saml-impl-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-security-api-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-security-impl-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-soap-api-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-soap-impl-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-storage-api-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-storage-impl-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-xmlsec-api-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-xmlsec-impl-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency pac4j-core-3.9.0.jar (via pac4j-saml)

Copy link
Author

Choose a reason for hiding this comment

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

I will live with the warnings to remove them I have to add more than 300 no-sense lines to the pom to manage these dependencies manually.

</exclusion>
<exclusion>
<groupId>org.springframework</groupId>
<artifactId>spring-jdbc</artifactId>
</exclusion>
<exclusion>
<groupId>org.springframework</groupId>
<artifactId>spring-orm</artifactId>
</exclusion>
<exclusion>
<groupId>org.springframework</groupId>
<artifactId>spring-tx</artifactId>
</exclusion>
<exclusion>
<groupId>org.springframework</groupId>
<artifactId>spring-core</artifactId>
Expand All @@ -104,17 +122,65 @@ under the License.
<groupId>org.slf4j</groupId>
<artifactId>jcl-over-slf4j</artifactId>
</exclusion>
<exclusion>
<groupId>org.bouncycastle</groupId>
<artifactId>bcprov-jdk15on</artifactId>
</exclusion>
<exclusion>
<groupId>org.dom4j</groupId>
<artifactId>dom4j</artifactId>
</exclusion>
<exclusion>
<groupId>commons-codec</groupId>
<artifactId>commons-codec</artifactId>
</exclusion>
<exclusion>
<groupId>org.bouncycastle</groupId>
<artifactId>bcprov-jdk15on</artifactId>
<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>
</exclusion>
<exclusion>
<groupId>commons-codec</groupId>
<artifactId>commons-codec</artifactId>
</exclusion>
<exclusion>
<groupId>org.apache.httpcomponents</groupId>
<artifactId>httpclient</artifactId>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</exclusion>
<exclusion>
<groupId>xalan</groupId>
<artifactId>xalan</artifactId>
</exclusion>
<exclusion>
<groupId>jakarta.activation</groupId>
<artifactId>jakarta.activation-api</artifactId>
</exclusion>
<exclusion>
<groupId>javax.annotation</groupId>
<artifactId>javax.annotation-api</artifactId>
</exclusion>
</exclusions>
</dependency>
Expand All @@ -129,21 +195,6 @@ under the License.
<artifactId>bouncycastle-api</artifactId>
<version>2.18</version>
</dependency>
<dependency>
<groupId>net.shibboleth.utilities</groupId>
<artifactId>java-support</artifactId>
<version>7.2.0</version>
<exclusions>
<exclusion>
<groupId>commons-codec</groupId>
<artifactId>commons-codec</artifactId>
</exclusion>
<exclusion>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
Expand Down Expand Up @@ -171,17 +222,29 @@ under the License.
</dependencies>
<dependencyManagement>
<dependencies>
<dependency>
<groupId>io.jenkins.tools.bom</groupId>
<artifactId>bom-2.249.x</artifactId>
<version>17</version>
<type>pom</type>
<scope>import</scope>
</dependency>
<dependency>
<groupId>org.cryptacular</groupId>
<artifactId>cryptacular</artifactId>
<version>1.2.4</version>
</dependency>
<!-- TODO: remove when CVE-2019-12400 is resolved -->
<dependency>
<groupId>org.apache.santuario</groupId>
<artifactId>xmlsec</artifactId>
<version>2.1.4</version>
</dependency>
<dependency>
<groupId>org.pac4j</groupId>
<artifactId>pac4j-saml</artifactId>
<!-- versions 4.x.x require JDK 11 -->
<version>3.9.0</version>
</dependency>
</dependencies>
</dependencyManagement>
<build>
Expand Down
17 changes: 13 additions & 4 deletions src/main/java/org/jenkinsci/plugins/saml/OpenSAMLWrapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@
import org.opensaml.core.config.InitializationService;
import org.pac4j.core.context.J2EContext;
import org.pac4j.core.context.WebContext;
import org.pac4j.core.http.callback.NoParameterCallbackUrlResolver;
import org.pac4j.core.http.url.DefaultUrlResolver;
import org.pac4j.saml.client.SAML2Client;
import org.pac4j.saml.config.SAML2Configuration;

import java.io.IOException;
import java.util.logging.Logger;

import static java.util.logging.Level.*;
Expand Down Expand Up @@ -91,9 +95,9 @@ protected WebContext createWebContext() {
* @return a SAML2Client object to interact with the IdP service.
*/
protected SAML2Client createSAML2Client() {
final SAML2ClientConfigurationCustom config = new SAML2ClientConfigurationCustom();
final SAML2Configuration config = new SAML2Configuration();
config.setIdentityProviderMetadataResource(new SamlFileResource(SamlSecurityRealm.getIDPMetadataFilePath()));
config.setDestinationBindingType(samlPluginConfig.getBinding());
config.setAuthnRequestBindingType(samlPluginConfig.getBinding());
config.setWantsAssertionsSigned(true);

SamlEncryptionData encryptionData = samlPluginConfig.getEncryptionData();
Expand Down Expand Up @@ -150,10 +154,15 @@ protected SAML2Client createSAML2Client() {
config.setServiceProviderMetadataResource(new SamlFileResource(SamlSecurityRealm.getSPMetadataFilePath()));
final SAML2Client saml2Client = new SAML2Client(config);
saml2Client.setCallbackUrl(samlPluginConfig.getConsumerServiceUrl());
saml2Client.init(createWebContext());
saml2Client.setCallbackUrlResolver(new NoParameterCallbackUrlResolver());
saml2Client.init();

if (LOG.isLoggable(FINE)) {
LOG.fine(saml2Client.getServiceProviderMetadataResolver().getMetadata());
try {
LOG.fine(saml2Client.getServiceProviderMetadataResolver().getMetadata());
} catch (IOException e) {
LOG.fine("Is not possible to show the metadata : " + e.getMessage());
}
}
return saml2Client;
}
Expand Down

This file was deleted.

53 changes: 51 additions & 2 deletions src/main/java/org/jenkinsci/plugins/saml/SamlFileResource.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,19 @@
package org.jenkinsci.plugins.saml;

import org.apache.commons.io.FileUtils;
import org.apache.commons.lang.NotImplementedException;
import org.pac4j.core.exception.TechnicalException;
import org.pac4j.core.io.WritableResource;
import org.springframework.core.io.Resource;
import org.springframework.core.io.WritableResource;

import javax.annotation.Nonnull;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.UnsupportedEncodingException;
import java.net.URI;
import java.net.URL;
import java.util.logging.Logger;

/**
Expand Down Expand Up @@ -61,11 +65,36 @@ public boolean exists() {
return getFile().exists();
}

@Override
public boolean isReadable() {
return getFile().canRead();
}

@Override
public boolean isOpen() {
return false;
}

@Override
public URL getURL() {
throw new NotImplementedException();
}

@Override
public URI getURI() {
throw new NotImplementedException();
}

@Override
public String getFilename() {
return fileName;
}

@Override
public String getDescription() {
return fileName;
}

@Override
public InputStream getInputStream() throws IOException {
return FileUtils.openInputStream(getFile());
Expand All @@ -76,8 +105,28 @@ public File getFile() {
return new File(fileName);
}

@Override
public long contentLength() {
return getFile().length();
}

@Override
public long lastModified() {
return getFile().lastModified();
}

@Override
public Resource createRelative(String s) {
throw new NotImplementedException();
}

@Override
public boolean isWritable() {
return getFile().canWrite();
}

@Override
public OutputStream getOutputStream() throws IOException {
return FileUtils.openOutputStream(getFile());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;
import org.pac4j.core.client.RedirectAction;
import org.pac4j.core.redirect.RedirectAction;
import org.pac4j.core.context.WebContext;
import org.pac4j.core.exception.HttpAction;
import org.pac4j.saml.client.SAML2Client;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import org.kohsuke.stapler.StaplerResponse;
import org.pac4j.saml.client.SAML2Client;

import java.io.IOException;

/**
* build the Service Provider(SP) metadata from the configuration.
*/
Expand All @@ -41,6 +43,12 @@ public SamlSPMetadataWrapper(SamlPluginConfig samlPluginConfig, StaplerRequest r
@Override
protected HttpResponse process() throws IllegalStateException {
final SAML2Client client = createSAML2Client();
return HttpResponses.plainText(client.getServiceProviderMetadataResolver().getMetadata());
String metadata = "";
try {
metadata = client.getServiceProviderMetadataResolver().getMetadata();
} catch (IOException e) {
throw new IllegalStateException(e);
}
return HttpResponses.text(metadata);
}
}
Loading