[JEP-227] Replace Acegi Security with Spring Security & upgrade Spring Framework (#4848)

Replacing Acegi Security with Spring Security

Co-authored-by: Tim Jacomb <[email protected]>
Co-authored-by: James Nord <[email protected]>

Author: 3 people

Jenkinsfile

@@ -43,7 +43,7 @@ for(j = 0; j < jdks.size(); j++) {
                                     "MAVEN_OPTS=-Xmx1536m -Xms512m"], buildType, jdk) {
                             // Actually run Maven!
                             // -Dmaven.repo.local=… tells Maven to create a subdir in the temporary directory for the local Maven repository
-                            def mvnCmd = "mvn -Pdebug -U -Dset.changelist help:evaluate -Dexpression=changelist -Doutput=$changelistF clean install ${runTests ? '-Dmaven.test.failure.ignore' : '-DskipTests'} -V -B -ntp -Dmaven.repo.local=$m2repo -e"
+                            def mvnCmd = "mvn -Pdebug -Pjapicmp -U -Dset.changelist help:evaluate -Dexpression=changelist -Doutput=$changelistF clean install ${runTests ? '-Dmaven.test.failure.ignore' : '-DskipTests'} -V -B -ntp -Dmaven.repo.local=$m2repo -e"
 
                             if(isUnix()) {
                                 sh mvnCmd
@@ -69,6 +69,7 @@ for(j = 0; j < jdks.size(); j++) {
                                              allowEmptyArchive: true, // in case we forgot to reincrementalify
                                              fingerprint: true
                         }
+                        publishHTML([allowMissing: true, alwaysLinkToLastBuild: false, includes: 'japicmp.html', keepAll: false, reportDir: 'core/target/japicmp', reportFiles: 'japicmp.html', reportName: 'API compatibility', reportTitles: 'japicmp report'])
                     }
                 }
         }

bom/pom.xml

@@ -42,12 +42,18 @@ THE SOFTWARE.
     <guavaVersion>11.0.1</guavaVersion>
     <slf4jVersion>1.7.30</slf4jVersion>
     <stapler.version>1.261</stapler.version>
-    <spring.version>2.5.6.SEC03</spring.version>
     <groovy.version>2.4.12</groovy.version>
   </properties>
 
   <dependencyManagement>
     <dependencies>
+      <dependency> <!-- https://docs.spring.io/spring-security/site/docs/5.4.0-M1/reference/html5/#getting-maven-no-boot -->
+        <groupId>org.springframework.security</groupId>
+        <artifactId>spring-security-bom</artifactId>
+        <version>5.4.1</version>
+        <type>pom</type>
+        <scope>import</scope>
+      </dependency>
       <dependency>
         <groupId>com.github.spotbugs</groupId>
         <artifactId>spotbugs-annotations</artifactId>
@@ -418,29 +424,6 @@ THE SOFTWARE.
         <version>1.1-beta-11</version>
       </dependency>
 
-      <!--Spring-->
-      <dependency>
-        <groupId>org.springframework</groupId>
-        <artifactId>spring-webmvc</artifactId>
-        <version>${spring.version}</version>
-      </dependency>
-      <dependency>
-        <groupId>org.springframework</groupId>
-        <artifactId>spring-core</artifactId>
-        <version>${spring.version}</version>
-      </dependency>
-      <dependency><!-- Jenkins core doesn't use it but JENKINS-3881 requires us to put it. -->
-        <groupId>org.springframework</groupId>
-        <artifactId>spring-aop</artifactId>
-        <version>${spring.version}</version>
-      </dependency>
-      <dependency>
-        <groupId>org.acegisecurity</groupId>
-        <artifactId>acegi-security</artifactId>
-        <version>1.0.7</version>
-      </dependency>
-
-
       <!-- Modules -->
       <dependency>
         <groupId>org.jenkins-ci.modules</groupId>

core/pom.xml

@@ -387,16 +387,12 @@ THE SOFTWARE.
       <artifactId>commons-jexl</artifactId>
     </dependency>
     <dependency>
-      <groupId>org.acegisecurity</groupId>
-      <artifactId>acegi-security</artifactId>
+      <groupId>org.springframework.security</groupId>
+      <artifactId>spring-security-web</artifactId>
       <exclusions>
         <exclusion>
           <groupId>org.springframework</groupId>
-          <artifactId>spring-remoting</artifactId>
-        </exclusion>
-        <exclusion>
-          <groupId>org.springframework</groupId>
-          <artifactId>spring-support</artifactId>
+          <artifactId>spring-jcl</artifactId>
         </exclusion>
       </exclusions>
     </dependency>
@@ -414,22 +410,6 @@ THE SOFTWARE.
       <groupId>org.fusesource.jansi</groupId>
       <artifactId>jansi</artifactId>
     </dependency>
-    <dependency>
-      <!--
-        for Grails spring bean builder.
-        Ideally we should be able to modify BeanBuilder so as not to depend on this.
-      -->
-      <groupId>org.springframework</groupId>
-      <artifactId>spring-webmvc</artifactId>
-    </dependency>
-    <dependency>
-      <groupId>org.springframework</groupId>
-      <artifactId>spring-core</artifactId>
-    </dependency>
-    <dependency><!-- Jenkins core doesn't use it but HUDSON-3811 requires us to put it. -->
-      <groupId>org.springframework</groupId>
-      <artifactId>spring-aop</artifactId>
-    </dependency>
     <dependency>
       <groupId>org.junit.jupiter</groupId>
       <artifactId>junit-jupiter-api</artifactId>
@@ -869,5 +849,41 @@ THE SOFTWARE.
         <maven.test.redirectTestOutputToFile>true</maven.test.redirectTestOutputToFile>
       </properties>
     </profile>
+    <profile>
+      <id>japicmp</id>
+      <build>
+        <plugins>
+          <plugin>
+            <groupId>com.github.siom79.japicmp</groupId>
+            <artifactId>japicmp-maven-plugin</artifactId>
+            <version>0.14.4-20200728.214757-1</version> <!-- TODO https://github.com/siom79/japicmp/pull/266 -->
+            <configuration>
+              <parameter>
+                <!-- see https://siom79.github.io/japicmp/MavenPlugin.html -->
+                <oldVersionPattern>\d+[.]\d+</oldVersionPattern>
+                <!-- <onlyModified>true</onlyModified> -->
+                <onlyBinaryIncompatible>true</onlyBinaryIncompatible>
+              </parameter>
+              <oldClassPathDependencies>
+                <dependency> <!-- provided, so not visible in flattened artifact -->
+                  <groupId>javax.servlet</groupId>
+                  <artifactId>javax.servlet-api</artifactId>
+                  <version>3.1.0</version>
+                  <scope>provided</scope>
+                </dependency>
+              </oldClassPathDependencies>
+            </configuration>
+            <executions>
+              <execution>
+                <phase>verify</phase>
+                <goals>
+                  <goal>cmp</goal>
+                </goals>
+              </execution>
+            </executions>
+          </plugin>
+        </plugins>
+      </build>
+    </profile>
   </profiles>
 </project>

core/src/main/java/hudson/DependencyRunner.java

@@ -52,7 +52,7 @@ public DependencyRunner(ProjectRunnable runnable) {
     }
 
     public void run() {
-        try (ACLContext ctx = ACL.as(ACL.SYSTEM)) {
+        try (ACLContext ctx = ACL.as2(ACL.SYSTEM2)) {
             Set<AbstractProject> topLevelProjects = new HashSet<>();
             // Get all top-level projects
             LOGGER.fine("assembling top level projects");

core/src/main/java/hudson/ExpressionFactory2.java

@@ -1,6 +1,5 @@
 package hudson;
 
-import org.acegisecurity.AcegiSecurityException;
 import org.apache.commons.jelly.JellyContext;
 import org.apache.commons.jelly.JellyException;
 import org.apache.commons.jelly.expression.Expression;
@@ -15,6 +14,7 @@
 import java.util.logging.Logger;
 import org.kohsuke.stapler.Stapler;
 import org.kohsuke.stapler.StaplerRequest;
+import org.springframework.security.access.AccessDeniedException;
 
 /**
  * {@link ExpressionFactory} so that security exception aborts the page rendering.
@@ -72,7 +72,7 @@ public Object evaluate(JellyContext context) {
                 CURRENT_CONTEXT.set(context);
                 JexlContext jexlContext = new JellyJexlContext( context );
                 return expression.evaluate(jexlContext);
-            } catch (AcegiSecurityException e) {
+            } catch (AccessDeniedException e) {
                 // let the security exception pass through
                 throw e;
             } catch (Exception e) {

core/src/main/java/hudson/ExtensionFinder.java

@@ -578,7 +578,7 @@ public <T> void onProvision(ProvisionInvocation<T> provision) {
                 // so that we invoke them before derived class one. This isn't specified in JSR-250 but implemented
                 // this way in Spring and what most developers would expect to happen.
 
-                final Set<Class> interfaces = ClassUtils.getAllInterfacesAsSet(instance);
+                final Set<Class<?>> interfaces = ClassUtils.getAllInterfacesAsSet(instance);
 
                 while (c != Object.class) {
                     Arrays.stream(c.getDeclaredMethods())
@@ -607,7 +607,7 @@ public <T> void onProvision(ProvisionInvocation<T> provision) {
      * This allows to introspect metadata for a method which is both declared in parent class and in implemented
      * interface(s). {@code interfaces} typically is obtained by {@link ClassUtils#getAllInterfacesAsSet}
      */
-    Collection<Method> getMethodAndInterfaceDeclarations(Method method, Collection<Class> interfaces) {
+    Collection<Method> getMethodAndInterfaceDeclarations(Method method, Collection<Class<?>> interfaces) {
         final List<Method> methods = new ArrayList<>();
         methods.add(method);
 

core/src/main/java/hudson/Functions.java

@@ -145,7 +145,6 @@
 import jenkins.model.ModelObjectWithChildren;
 import jenkins.model.ModelObjectWithContextMenu;
 
-import org.acegisecurity.AccessDeniedException;
 import org.apache.commons.jelly.JellyContext;
 import org.apache.commons.jelly.JellyTagException;
 import org.apache.commons.jelly.Script;
@@ -174,6 +173,7 @@
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.accmod.restrictions.DoNotUse;
+import org.springframework.security.access.AccessDeniedException;
 
 /**
  * Utility functions used in views.
@@ -1770,7 +1770,7 @@ public static String toCCStatus(Item i) {
      * Checks if the current user is anonymous.
      */
     public static boolean isAnonymous() {
-        return ACL.isAnonymous(Jenkins.getAuthentication());
+        return ACL.isAnonymous2(Jenkins.getAuthentication2());
     }
 
     /**

core/src/main/java/hudson/PluginManager.java

@@ -69,7 +69,6 @@
 import jenkins.util.xml.RestrictiveEntityResolver;
 import net.sf.json.JSONArray;
 import net.sf.json.JSONObject;
-import org.acegisecurity.Authentication;
 import org.apache.commons.fileupload.FileItem;
 import org.apache.commons.fileupload.FileUploadException;
 import org.apache.commons.fileupload.disk.DiskFileItemFactory;
@@ -156,6 +155,7 @@
 
 import static hudson.init.InitMilestone.*;
 import static java.util.logging.Level.*;
+import org.springframework.security.core.Authentication;
 
 /**
  * Manages {@link PluginWrapper}s.
@@ -884,7 +884,7 @@ public void dynamicLoad(File arc) throws IOException, InterruptedException, Rest
      */
     @Restricted(NoExternalUse.class)
     public void dynamicLoad(File arc, boolean removeExisting, @CheckForNull List<PluginWrapper> batch) throws IOException, InterruptedException, RestartRequiredException {
-        try (ACLContext context = ACL.as(ACL.SYSTEM)) {
+        try (ACLContext context = ACL.as2(ACL.SYSTEM2)) {
             LOGGER.log(FINE, "Attempting to dynamic load {0}", arc);
             PluginWrapper p = null;
             String sn;
@@ -951,7 +951,7 @@ public void dynamicLoad(File arc, boolean removeExisting, @CheckForNull List<Plu
 
     @Restricted(NoExternalUse.class)
     public void start(List<PluginWrapper> plugins) throws Exception {
-      try (ACLContext context = ACL.as(ACL.SYSTEM)) {
+      try (ACLContext context = ACL.as2(ACL.SYSTEM2)) {
         Map<String, PluginWrapper> pluginsByName = plugins.stream().collect(Collectors.toMap(p -> p.getShortName(), p -> p));
 
         // recalculate dependencies of plugins optionally depending the newly deployed ones.
@@ -1548,7 +1548,7 @@ private List<Future<UpdateCenter.UpdateCenterJob>> install(@NonNull Collection<S
             installJobs.add(updateCenter.addJob(updateCenter.new CompleteBatchJob(batch, start, correlationId)));
         }
 
-        final Authentication currentAuth = Jenkins.getAuthentication();
+        final Authentication currentAuth = Jenkins.getAuthentication2();
 
         if (!jenkins.getInstallState().isSetupComplete()) {
             jenkins.setInstallState(InstallState.INITIAL_PLUGINS_INSTALLING);
@@ -1578,7 +1578,7 @@ public void run() {
                     }
                     updateCenter.persistInstallStatus();
                     if(!failures) {
-                        try (ACLContext acl = ACL.as(currentAuth)) {
+                        try (ACLContext acl = ACL.as2(currentAuth)) {
                             InstallUtil.proceedToNextStateFrom(InstallState.INITIAL_PLUGINS_INSTALLING);
                         }
                     }

core/src/main/java/hudson/WebAppMain.java

@@ -434,7 +434,7 @@ public FileAndDescription getHomeDir(ServletContextEvent event) {
     }
 
     public void contextDestroyed(ServletContextEvent event) {
-        try (ACLContext old = ACL.as(ACL.SYSTEM)) {
+        try (ACLContext old = ACL.as2(ACL.SYSTEM2)) {
             Jenkins instance = Jenkins.getInstanceOrNull();
             try {
                 if (instance != null) {

core/src/main/java/hudson/cli/BuildCommand.java

@@ -166,7 +166,7 @@ protected int run() throws Exception {
             throw new IllegalStateException(msg);
         }
 
-        Queue.Item item = ParameterizedJobMixIn.scheduleBuild2(job, 0, new CauseAction(new CLICause(Jenkins.getAuthentication().getName())), a);
+        Queue.Item item = ParameterizedJobMixIn.scheduleBuild2(job, 0, new CauseAction(new CLICause(Jenkins.getAuthentication2().getName())), a);
         QueueTaskFuture<? extends Run<?,?>> f = item != null ? (QueueTaskFuture)item.getFuture() : null;
 
         if (wait || sync || follow) {