[FIXED JENKINS-18285] Merge the feature branch

jenkinsci · Jun 12, 2013 · 2d09a0e · 2d09a0e
2 parents 3c01366 + b89387c
commit 2d09a0e
Show file tree

Hide file tree

Showing 24 changed files with 403 additions and 40 deletions.
diff --git a/changelog.html b/changelog.html
@@ -61,6 +61,9 @@
   <li class=bug>
     “Projects tied to slave” shows unrelated Maven module jobs.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-17451">issue 17451</a>)
+  <li class=rfe>
+    Executors running the builds can be now a subject of access control.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-18285">issue 18285</a>)
 </ul>
 </div><!--=TRUNK-END=-->
 

diff --git a/core/pom.xml b/core/pom.xml
@@ -192,13 +192,7 @@ THE SOFTWARE.
     <dependency>
       <groupId>com.infradna.tool</groupId>
       <artifactId>bridge-method-annotation</artifactId>
-      <version>1.4</version>
-      <exclusions> <!-- https://github.com/infradna/bridge-method-injector/issues/1 -->
-        <exclusion>
-          <artifactId>annotation-indexer</artifactId>
-          <groupId>org.jvnet.hudson</groupId>
-        </exclusion>
-      </exclusions>
+      <version>1.8</version>
     </dependency>
 
     <dependency><!-- until we get this version through Stapler -->
@@ -655,7 +649,7 @@ THE SOFTWARE.
       <plugin>
         <groupId>com.infradna.tool</groupId>
         <artifactId>bridge-method-injector</artifactId>
-        <!-- version specified in grandparent pom -->
+        <version>1.5-SNAPSHOT</version>
         <executions>
           <execution>
             <goals>

diff --git a/core/src/main/java/hudson/ExtensionListView.java b/core/src/main/java/hudson/ExtensionListView.java
@@ -142,7 +142,7 @@ public void clear() {
             }
 
             @Override
-            public T[] toArray(T[] array) {
+            public <T> T[] toArray(T[] array) {
                 return storage().toArray(array);
             }
 

diff --git a/core/src/main/java/hudson/model/AbstractProject.java b/core/src/main/java/hudson/model/AbstractProject.java
@@ -90,8 +90,12 @@
 import jenkins.scm.DefaultSCMCheckoutStrategyImpl;
 import jenkins.scm.SCMCheckoutStrategy;
 import jenkins.scm.SCMCheckoutStrategyDescriptor;
+import jenkins.security.ProjectAuthenticator;
+import jenkins.security.ProjectAuthenticatorConfiguration;
+import jenkins.security.ProjectAuthenticatorConfiguration;
 import jenkins.util.TimeDuration;
 import net.sf.json.JSONObject;
+import org.acegisecurity.Authentication;
 import org.acegisecurity.context.SecurityContext;
 import org.acegisecurity.context.SecurityContextHolder;
 import org.kohsuke.accmod.Restricted;
@@ -1174,6 +1178,13 @@ public final Task getOwnerTask() {
         return this;
     }
 
+    /**
+     * Let the identity determined by {@link ProjectAuthenticator}.
+     */
+    public Authentication getIdentity() {
+        return ProjectAuthenticatorConfiguration.get().authenticate(this);
+    }
+
     /**
      * {@inheritDoc}
      *

diff --git a/core/src/main/java/hudson/model/Executor.java b/core/src/main/java/hudson/model/Executor.java
@@ -38,6 +38,8 @@
 import jenkins.model.InterruptedBuildAction;
 import jenkins.model.Jenkins;
 import org.acegisecurity.Authentication;
+import org.acegisecurity.context.SecurityContext;
+import org.acegisecurity.context.SecurityContextHolder;
 import org.acegisecurity.providers.anonymous.AnonymousAuthenticationToken;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.HttpResponses;
@@ -236,10 +238,16 @@ public void run() {
                             ((Actionable) executable).addAction(action);
                         }
                     }
-                    setName(threadName+" : executing "+executable.toString());
-                    if (LOGGER.isLoggable(FINE))
-                        LOGGER.log(FINE, getName()+" is now executing "+executable);
-                    queue.execute(executable, task);
+
+                    final SecurityContext savedContext = ACL.impersonate(Tasks.getIdentityOf(task));
+                    try {
+                        setName(threadName + " : executing " + executable.toString());
+                        if (LOGGER.isLoggable(FINE))
+                            LOGGER.log(FINE, getName()+" is now executing "+executable);
+                        queue.execute(executable, task);
+                    } finally {
+                        SecurityContextHolder.setContext(savedContext);
+                    }
                 } catch (Throwable e) {
                     // for some reason the executor died. this is really
                     // a bug in the code, but we don't want the executor to die,

diff --git a/core/src/main/java/hudson/model/Fingerprint.java b/core/src/main/java/hudson/model/Fingerprint.java
@@ -794,21 +794,13 @@ public Iterator<FingerprintFacet> iterator() {
 
             @Override
             public boolean add(FingerprintFacet e) {
-                try {
-                    facets.add(e);
-                    return true;
-                } catch (IOException x) {
-                    throw new Error(x);
-                }
+                facets.add(e);
+                return true;
             }
 
             @Override
             public boolean remove(Object o) {
-                try {
-                    return facets.remove((FingerprintFacet)o);
-                } catch (IOException x) {
-                    throw new Error(x);
-                }
+                return facets.remove(o);
             }
 
             @Override

diff --git a/core/src/main/java/hudson/model/Node.java b/core/src/main/java/hudson/model/Node.java
@@ -62,6 +62,7 @@
 import jenkins.model.Jenkins;
 import jenkins.util.io.OnMaster;
 import net.sf.json.JSONObject;
+import org.acegisecurity.Authentication;
 import org.jvnet.localizer.Localizable;
 import org.kohsuke.stapler.BindInterceptor;
 import org.kohsuke.stapler.Stapler;
@@ -326,6 +327,13 @@ public CauseOfBlockage canTake(Queue.BuildableItem item) {
         if(l==null && getMode()== Mode.EXCLUSIVE)
             return CauseOfBlockage.fromMessage(Messages._Node_BecauseNodeIsReserved(getNodeName()));   // this node is reserved for tasks that are tied to it
 
+        Authentication identity = item.task.getIdentity();
+        if (!getACL().hasPermission(identity,AbstractProject.BUILD)) {
+            // doesn't have a permission
+            // TODO: does it make more sense to define a separate permission?
+            return CauseOfBlockage.fromMessage(Messages._Node_LackingBuildPermission(identity.getName(),getNodeName()));
+        }
+
         // Check each NodeProperty to see whether they object to this node
         // taking the task
         for (NodeProperty prop: getNodeProperties()) {

diff --git a/core/src/main/java/hudson/model/Run.java b/core/src/main/java/hudson/model/Run.java
@@ -1523,7 +1523,7 @@ public Map<Object,Object> getAttributes() {
     }
 
     /**
-     * Used in {@link RunExecution#run} to indicates that a fatal error in a build
+     * Used in {@link Run.RunExecution#run} to indicates that a fatal error in a build
      * is reported to {@link BuildListener} and the build should be simply aborted
      * without further recording a stack trace.
      */

diff --git a/core/src/main/java/hudson/model/queue/AbstractQueueTask.java b/core/src/main/java/hudson/model/queue/AbstractQueueTask.java
@@ -26,6 +26,8 @@
 
 import hudson.model.Queue;
 import hudson.model.Queue.Task;
+import hudson.security.ACL;
+import org.acegisecurity.Authentication;
 
 import java.util.Collection;
 import java.util.Collections;
@@ -57,4 +59,12 @@ public CauseOfBlockage getCauseOfBlockage() {
     public Object getSameNodeConstraint() {
         return null;
     }
+
+    /**
+     * This default implementation is the historical behaviour, but this is no longer desirable. Please override.
+     * See {@link SubTask#getIdentity()} for the contract.
+     */
+    public Authentication getIdentity() {
+        return ACL.SYSTEM;
+    }
 }
diff --git a/core/src/main/java/hudson/model/queue/AbstractSubTask.java b/core/src/main/java/hudson/model/queue/AbstractSubTask.java
@@ -26,6 +26,8 @@
 import hudson.model.Label;
 import hudson.model.Node;
 import hudson.model.ResourceList;
+import hudson.security.ACL;
+import org.acegisecurity.Authentication;
 
 /**
  * Partial default implementation of {@link SubTask} to avoid
@@ -53,4 +55,8 @@ public Object getSameNodeConstraint() {
     public ResourceList getResourceList() {
         return new ResourceList();
     }
+
+    public Authentication getIdentity() {
+        return getOwnerTask().getIdentity();
+    }
 }
diff --git a/core/src/main/java/hudson/model/queue/MappingWorksheet.java b/core/src/main/java/hudson/model/queue/MappingWorksheet.java
@@ -25,6 +25,7 @@
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
+import hudson.model.AbstractProject;
 import hudson.model.Computer;
 import hudson.model.Executor;
 import hudson.model.Label;
@@ -35,6 +36,7 @@
 import hudson.model.Queue.JobOffer;
 import hudson.model.Queue.Task;
 import hudson.model.labels.LabelAssignmentAction;
+import hudson.security.ACL;
 
 import java.util.AbstractList;
 import java.util.ArrayList;
@@ -57,14 +59,17 @@
  * which determines where each {@link SubTask} gets executed.
  *
  * <p>
- * This mapping is done under two constraints:
+ * This mapping is done under the following constraints:
  *
  * <ul>
  * <li>
  * "Same node" constraint. Some of the subtasks need to be co-located on the same node.
  * See {@link SubTask#getSameNodeConstraint()}
  * <li>
  * Label constraint. {@link SubTask}s can specify that it can be only run on nodes that has the label.
+ * <li>
+ * Permission constraint. {@link SubTask}s have {@linkplain SubTask#getIdentity() identities} that need to have
+ * permissions to build on the node.
  * </ul>
  *
  * <p>
@@ -111,21 +116,33 @@ public final class ExecutorChunk extends ReadOnlyList<ExecutorSlot> {
         public final int index;
         public final Computer computer;
         public final Node node;
+        public final ACL nodeAcl;
 
         private ExecutorChunk(List<ExecutorSlot> base, int index) {
             super(base);
             this.index = index;
             assert !base.isEmpty();
             computer = base.get(0).getExecutor().getOwner();
             node = computer.getNode();
+            nodeAcl = node.getACL();
         }
 
         /**
          * Is this executor chunk and the given work chunk compatible? Can the latter be run on the former?
          */
         public boolean canAccept(WorkChunk c) {
-            return this.size() >= c.size()
-                && (c.assignedLabel==null || c.assignedLabel.contains(node));
+            if (this.size()<c.size())
+                return false;   // too small compared towork
+
+            if (c.assignedLabel!=null && !c.assignedLabel.contains(node))
+                return false;   // label mismatch
+
+            for (SubTask task : c) {
+                if (!nodeAcl.hasPermission(task.getIdentity(), AbstractProject.BUILD))
+                    return false;   // tasks don't have a permission to run on this node
+            }
+
+            return true;
         }
 
         /**
@@ -154,6 +171,9 @@ private void execute(WorkChunk wc, WorkUnitContext wuc) {
         }
     }
 
+    /**
+     * {@link SubTask}s that need to run on the same node.
+     */
     public class WorkChunk extends ReadOnlyList<SubTask> {
         public final int index;
 

diff --git a/core/src/main/java/hudson/model/queue/QueueTaskFilter.java b/core/src/main/java/hudson/model/queue/QueueTaskFilter.java
@@ -30,6 +30,7 @@
 import hudson.model.Queue.Executable;
 import hudson.model.Queue.Task;
 import hudson.model.ResourceList;
+import org.acegisecurity.Authentication;
 
 import java.io.IOException;
 import java.util.Collection;
@@ -118,4 +119,8 @@ public final Task getOwnerTask() {
     public Object getSameNodeConstraint() {
         return base.getSameNodeConstraint();
     }
+
+    public Authentication getIdentity() {
+        return base.getIdentity();
+    }
 }
diff --git a/core/src/main/java/hudson/model/queue/SubTask.java b/core/src/main/java/hudson/model/queue/SubTask.java
@@ -29,7 +29,10 @@
 import hudson.model.Queue.Executable;
 import hudson.model.Queue.Task;
 import hudson.model.ResourceActivity;
+import hudson.security.ACL;
+import org.acegisecurity.Authentication;
 
+import javax.annotation.Nonnull;
 import java.io.IOException;
 
 /**
@@ -83,4 +86,17 @@ public interface SubTask extends ResourceActivity {
      * colocation constraint.
      */
     Object getSameNodeConstraint();
+
+    /**
+     * Returns the identity that this task carries when it runs, for the purpose of access control.
+     *
+     * When the task execution touches other objects inside Jenkins, the access control is performed
+     * based on whether this {@link Authentication} is allowed to use them. Implementers, if you are unsure,
+     * return the identity of the user who queued the task, or {@link ACL#SYSTEM} to bypass the access control
+     * and run as the super user.
+     *
+     * @since 1.520
+     * @see Tasks#getIdentityOf(SubTask)
+     */
+    @Nonnull Authentication getIdentity();
 }
diff --git a/core/src/main/java/hudson/model/queue/Tasks.java b/core/src/main/java/hudson/model/queue/Tasks.java
@@ -24,6 +24,8 @@
 package hudson.model.queue;
 
 import hudson.model.Queue.Task;
+import hudson.security.ACL;
+import org.acegisecurity.Authentication;
 
 import java.util.Collection;
 import java.util.Collections;
@@ -83,4 +85,21 @@ public static Task getOwnerTaskOf(SubTask t) {
             return (Task)t;
         }
     }
+
+    /**
+     * A pointless function to work around what appears to be a HotSpot problem. See JENKINS-5756 and bug 6933067
+     * on BugParade for more details.
+     */
+    private static Authentication _getIdentityOf(SubTask t) {
+        return t.getIdentity();
+    }
+
+    public static Authentication getIdentityOf(SubTask t) {
+        try {
+            return _getIdentityOf(t);
+        } catch (AbstractMethodError e) {
+            return ACL.SYSTEM;
+        }
+    }
+
 }
diff --git a/core/src/main/java/hudson/security/GlobalSecurityConfiguration.java b/core/src/main/java/hudson/security/GlobalSecurityConfiguration.java
@@ -50,6 +50,8 @@
 /**
  * Security configuration.
  *
+ * For historical reasons, most of the actual configuration values are stored in {@link Jenkins}.
+ *
  * @author Kohsuke Kawaguchi
  */
 @Extension(ordinal = Integer.MAX_VALUE - 210)

diff --git a/core/src/main/java/hudson/util/CopyOnWriteList.java b/core/src/main/java/hudson/util/CopyOnWriteList.java
@@ -138,7 +138,7 @@ public void clear() {
         this.core = new ArrayList<E>();
     }
 
-    public E[] toArray(E[] array) {
+    public <E> E[] toArray(E[] array) {
         return core.toArray(array);
     }