fixed a concurrency bug in the impersonation code

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

@@ -36,6 +36,7 @@ import java.util.Collection;
 import java.util.logging.Logger;
 
 import org.acegisecurity.Authentication;
+import org.acegisecurity.context.SecurityContext;
 import org.acegisecurity.context.SecurityContextHolder;
 
 /**
@@ -54,9 +55,7 @@ public class DependencyRunner implements Runnable {
     }
 
     public void run() {
-        Authentication saveAuth = SecurityContextHolder.getContext().getAuthentication();
-        SecurityContextHolder.getContext().setAuthentication(ACL.SYSTEM);
-
+        SecurityContext oldContext = ACL.impersonate(ACL.SYSTEM);
         try {
             Set<AbstractProject> topLevelProjects = new HashSet<AbstractProject>();
             // Get all top-level projects
@@ -74,7 +73,7 @@ public class DependencyRunner implements Runnable {
                 runnable.run(p);
             }
         } finally {
-            SecurityContextHolder.getContext().setAuthentication(saveAuth);
+            SecurityContextHolder.setContext(oldContext);
         }
     }
 

core/src/main/java/hudson/model/AsyncAperiodicWork.java

@@ -31,6 +31,7 @@ import java.io.IOException;
 import java.util.logging.Level;
 
 import jenkins.model.Jenkins;
+import org.acegisecurity.context.SecurityContext;
 import org.acegisecurity.context.SecurityContextHolder;
 
 /**
@@ -70,8 +71,8 @@ public abstract class AsyncAperiodicWork extends AperiodicWork {
 
                     StreamTaskListener l = createListener();
                     try {
-                        SecurityContextHolder.getContext().setAuthentication(ACL.SYSTEM);
-                        
+                        ACL.impersonate(ACL.SYSTEM);
+
                         execute(l);
                     } catch (IOException e) {
                         e.printStackTrace(l.fatalError(e.getMessage()));

core/src/main/java/hudson/model/AsyncPeriodicWork.java

@@ -3,6 +3,7 @@ package hudson.model;
 import hudson.security.ACL;
 import hudson.util.StreamTaskListener;
 import jenkins.model.Jenkins;
+import org.acegisecurity.context.SecurityContext;
 import org.acegisecurity.context.SecurityContextHolder;
 
 import java.io.File;
@@ -47,8 +48,8 @@ public abstract class AsyncPeriodicWork extends PeriodicWork {
 
                     StreamTaskListener l = createListener();
                     try {
-                        SecurityContextHolder.getContext().setAuthentication(ACL.SYSTEM);
-                        
+                        ACL.impersonate(ACL.SYSTEM);
+
                         execute(l);
                     } catch (IOException e) {
                         e.printStackTrace(l.fatalError(e.getMessage()));

core/src/main/java/hudson/model/DependencyGraph.java

@@ -84,12 +84,9 @@ public class DependencyGraph implements Comparator<AbstractProject> {
     public void build() {
         // Set full privileges while computing to avoid missing any projects the current user cannot see.
         // Use setContext (NOT getContext().setAuthentication()) so we don't affect concurrent threads for same HttpSession.
-        SecurityContext saveCtx = SecurityContextHolder.getContext();
+        SecurityContext saveCtx = ACL.impersonate(ACL.SYSTEM);
         try {
             this.computationalData = new HashMap<Class<?>, Object>();
-            NotSerilizableSecurityContext system = new NotSerilizableSecurityContext();
-            system.setAuthentication(ACL.SYSTEM);
-            SecurityContextHolder.setContext(system);
             for( AbstractProject p : getAllProjects() )
                 p.buildDependencyGraph(this);
 

core/src/main/java/hudson/model/Executor.java

@@ -38,6 +38,7 @@ import hudson.security.ACL;
 import jenkins.model.InterruptedBuildAction;
 import jenkins.model.Jenkins;
 import org.acegisecurity.Authentication;
+import org.acegisecurity.context.SecurityContext;
 import org.acegisecurity.providers.anonymous.AnonymousAuthenticationToken;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.HttpResponses;
@@ -175,7 +176,7 @@ public class Executor extends Thread implements ModelObject {
     @Override
     public void run() {
         // run as the system user. see ACL.SYSTEM for more discussion about why this is somewhat broken
-        SecurityContextHolder.getContext().setAuthentication(ACL.SYSTEM);
+        ACL.impersonate(ACL.SYSTEM);
 
         try {
             finishTime = System.currentTimeMillis();

core/src/main/java/hudson/model/UpdateCenter.java

@@ -48,6 +48,7 @@ import hudson.util.XStream2;
 import jenkins.RestartRequiredException;
 import jenkins.model.Jenkins;
 import org.acegisecurity.Authentication;
+import org.acegisecurity.context.SecurityContext;
 import org.apache.commons.io.input.CountingInputStream;
 import org.apache.commons.io.output.NullOutputStream;
 import org.jvnet.localizer.Localizable;
@@ -1121,13 +1122,14 @@ public class UpdateCenter extends AbstractModelObject implements Saveable {
 
             // if this is a bundled plugin, make sure it won't get overwritten
             PluginWrapper pw = plugin.getInstalled();
-            if (pw!=null && pw.isBundled())
+            if (pw!=null && pw.isBundled()) {
+                SecurityContext oldContext = ACL.impersonate(ACL.SYSTEM);
                 try {
-                    SecurityContextHolder.getContext().setAuthentication(ACL.SYSTEM);
                     pw.doPin();
                 } finally {
-                    SecurityContextHolder.clearContext();
+                    SecurityContextHolder.setContext(oldContext);
                 }
+            }
 
             if (dynamicLoad) {
                 try {

core/src/main/java/hudson/security/ACL.java

@@ -26,6 +26,8 @@ package hudson.security;
 import jenkins.model.Jenkins;
 import org.acegisecurity.AccessDeniedException;
 import org.acegisecurity.Authentication;
+import org.acegisecurity.context.SecurityContext;
+import org.acegisecurity.context.SecurityContextHolder;
 import org.acegisecurity.providers.UsernamePasswordAuthenticationToken;
 import org.acegisecurity.acls.sid.PrincipalSid;
 import org.acegisecurity.acls.sid.Sid;
@@ -113,4 +115,22 @@ public abstract class ACL {
      * the user who triggered a build.)
      */
     public static final Authentication SYSTEM = new UsernamePasswordAuthenticationToken("SYSTEM","SYSTEM");
+
+    /**
+     * Changes the {@link Authentication} associated with the current thread
+     * to the specified one, and returns  the previous security context.
+     * 
+     * <p>
+     * When the impersonation is over, be sure to restore the previous authentication
+     * via {@code SecurityContextHolder.setContext(returnValueFromThisMethod)}.
+     * 
+     * <p>
+     * We need to create a new {@link SecurityContext} instead of {@link SecurityContext#setAuthentication(Authentication)}
+     * because the same {@link SecurityContext} object is reused for all the concurrent requests from the same session.
+     */
+    public static SecurityContext impersonate(Authentication auth) {
+        SecurityContext old = SecurityContextHolder.getContext();
+        SecurityContextHolder.setContext(new NotSerilizableSecurityContext(ACL.SYSTEM));
+        return old;
+    }
 }

core/src/main/java/hudson/triggers/SafeTimerTask.java

@@ -23,6 +23,7 @@
  */
 package hudson.triggers;
 
+import org.acegisecurity.context.SecurityContext;
 import org.acegisecurity.context.SecurityContextHolder;
 
 import java.util.Timer;
@@ -48,14 +49,13 @@ public abstract class SafeTimerTask extends TimerTask {
     public final void run() {
         // background activity gets system credential,
         // just like executors get it.
-        SecurityContextHolder.getContext().setAuthentication(ACL.SYSTEM);
-
+        SecurityContext oldContext = ACL.impersonate(ACL.SYSTEM);
         try {
             doRun();
         } catch(Throwable t) {
             LOGGER.log(Level.SEVERE, "Timer task "+this+" failed",t);
         } finally {
-            SecurityContextHolder.clearContext();
+            SecurityContextHolder.setContext(oldContext);
         }
     }
 

core/src/main/java/jenkins/model/Jenkins.java

@@ -200,6 +200,7 @@ import org.acegisecurity.AcegiSecurityException;
 import org.acegisecurity.Authentication;
 import org.acegisecurity.GrantedAuthority;
 import org.acegisecurity.GrantedAuthorityImpl;
+import org.acegisecurity.context.SecurityContext;
 import org.acegisecurity.context.SecurityContextHolder;
 import org.acegisecurity.providers.anonymous.AnonymousAuthenticationToken;
 import org.acegisecurity.ui.AbstractProcessingFilter;
@@ -702,7 +703,7 @@ public class Jenkins extends AbstractCIBase implements ModifiableItemGroup<TopLe
         long start = System.currentTimeMillis();
         
     	// As Jenkins is starting, grant this process full control
-    	SecurityContextHolder.getContext().setAuthentication(ACL.SYSTEM);
+        ACL.impersonate(ACL.SYSTEM);
         try {
             this.root = root;
             this.servletContext = context;
@@ -826,7 +827,7 @@ public class Jenkins extends AbstractCIBase implements ModifiableItemGroup<TopLe
             protected void runTask(Task task) throws Exception {
                 if (is!=null && is.skipInitTask(task))  return;
 
-                SecurityContextHolder.getContext().setAuthentication(ACL.SYSTEM);   // full access in the initialization thread
+                ACL.impersonate(ACL.SYSTEM); // full access in the initialization thread
                 String taskName = task.getDisplayName();
 
                 Thread t = Thread.currentThread();
@@ -2912,7 +2913,7 @@ public class Jenkins extends AbstractCIBase implements ModifiableItemGroup<TopLe
             @Override
             public void run() {
                 try {
-                    SecurityContextHolder.getContext().setAuthentication(ACL.SYSTEM);
+                    ACL.impersonate(ACL.SYSTEM);
                     reload();
                 } catch (Exception e) {
                     LOGGER.log(SEVERE,"Failed to reload Jenkins config",e);
@@ -3081,7 +3082,7 @@ public class Jenkins extends AbstractCIBase implements ModifiableItemGroup<TopLe
             @Override
             public void run() {
                 try {
-                    SecurityContextHolder.getContext().setAuthentication(ACL.SYSTEM);
+                    ACL.impersonate(ACL.SYSTEM);
 
                     // give some time for the browser to load the "reloading" page
                     Thread.sleep(5000);
@@ -3113,7 +3114,7 @@ public class Jenkins extends AbstractCIBase implements ModifiableItemGroup<TopLe
             @Override
             public void run() {
                 try {
-                    SecurityContextHolder.getContext().setAuthentication(ACL.SYSTEM);
+                    ACL.impersonate(ACL.SYSTEM);
 
                     // Wait 'til we have no active executors.
                     doQuietDown(true, 0);
@@ -3175,7 +3176,7 @@ public class Jenkins extends AbstractCIBase implements ModifiableItemGroup<TopLe
             @Override
             public void run() {
                 try {
-                    SecurityContextHolder.getContext().setAuthentication(ACL.SYSTEM);
+                    ACL.impersonate(ACL.SYSTEM);
                     LOGGER.severe(String.format("Shutting down VM as requested by %s from %s",
                                                 exitUser, exitAddr));
                     // Wait 'til we have no active executors.

core/src/main/java/jenkins/security/ApiTokenFilter.java

 package jenkins.security;
 
 import hudson.model.User;
+import hudson.security.ACL;
 import hudson.util.Scrambler;
+import org.acegisecurity.context.SecurityContext;
 import org.acegisecurity.context.SecurityContextHolder;
 
 import javax.servlet.Filter;
@@ -47,12 +49,12 @@ public class ApiTokenFilter implements Filter {
                 if (t!=null && t.matchesPassword(password)) {
                     // even if we fail to match the password, we aren't rejecting it.
                     // as the user might be passing in a real password.
-                    SecurityContextHolder.getContext().setAuthentication(u.impersonate());
+                    SecurityContext oldContext = ACL.impersonate(u.impersonate());
                     try {
                         chain.doFilter(request,response);
                         return;
                     } finally {
-                        SecurityContextHolder.clearContext();
+                        SecurityContextHolder.setContext(oldContext);
                     }
                 }
             }

test/src/main/java/org/jvnet/hudson/test/HudsonTestCase.java

@@ -131,6 +131,7 @@ import net.sourceforge.htmlunit.corejs.javascript.ContextFactory.Listener;
 import org.acegisecurity.AuthenticationException;
 import org.acegisecurity.BadCredentialsException;
 import org.acegisecurity.GrantedAuthority;
+import org.acegisecurity.context.SecurityContext;
 import org.acegisecurity.context.SecurityContextHolder;
 import org.acegisecurity.userdetails.UserDetails;
 import org.acegisecurity.userdetails.UsernameNotFoundException;
@@ -394,7 +395,7 @@ public abstract class HudsonTestCase extends TestCase implements RootAction {
     protected void runTest() throws Throwable {
         System.out.println("=== Starting "+ getClass().getSimpleName() + "." + getName());
         // so that test code has all the access to the system
-        SecurityContextHolder.getContext().setAuthentication(ACL.SYSTEM);
+        ACL.impersonate(ACL.SYSTEM);
 
         try {
             super.runTest();

test/src/main/java/org/jvnet/hudson/test/JenkinsRule.java

@@ -121,6 +121,7 @@ import net.sourceforge.htmlunit.corejs.javascript.ContextFactory;
 import org.acegisecurity.AuthenticationException;
 import org.acegisecurity.BadCredentialsException;
 import org.acegisecurity.GrantedAuthority;
+import org.acegisecurity.context.SecurityContext;
 import org.acegisecurity.context.SecurityContextHolder;
 import org.acegisecurity.userdetails.UserDetails;
 import org.acegisecurity.userdetails.UsernameNotFoundException;
@@ -431,7 +432,7 @@ public class JenkinsRule implements TestRule, RootAction {
                 try {
                     System.out.println("=== Starting " + testDescription.getDisplayName());
                     // so that test code has all the access to the system
-                    SecurityContextHolder.getContext().setAuthentication(ACL.SYSTEM);
+                    ACL.impersonate(ACL.SYSTEM);
                     try {
                         base.evaluate();
                     } catch (Throwable th) {

test/src/test/java/hudson/model/DependencyGraphTest.java

@@ -29,6 +29,8 @@ import hudson.tasks.MailMessageIdAction;
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
+
+import org.acegisecurity.context.SecurityContext;
 import org.acegisecurity.context.SecurityContextHolder;
 import org.jvnet.hudson.test.HudsonTestCase;
 import org.jvnet.hudson.test.Bug;
@@ -114,7 +116,7 @@ public class DependencyGraphTest extends HudsonTestCase {
         hudson.rebuildDependencyGraph();
         try {
             // Switch to full access to check results:
-            SecurityContextHolder.getContext().setAuthentication(ACL.SYSTEM);
+            ACL.impersonate(ACL.SYSTEM);
             // @LocalData for this test has jobs w/o anonymous Item.READ
             AbstractProject up = (AbstractProject)hudson.getItem("hiddenUpstream");
             assertNotNull("hiddenUpstream project not found", up);