[FIXED JENKINS-22769] ItemListener callbacks should run as SYSTEM sin…

…ce they sometimes do ACL-checked calls. (cherry picked from commit c04cdcd) Conflicts: changelog.html
jenkinsci · Nov 8, 2014 · 0efd811 · 0efd811
1 parent 8fbd44d
commit 0efd811
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 15 deletions.
diff --git a/core/src/main/java/hudson/model/listeners/ItemListener.java b/core/src/main/java/hudson/model/listeners/ItemListener.java
@@ -26,10 +26,10 @@
 import hudson.ExtensionPoint;
 import hudson.ExtensionList;
 import hudson.Extension;
-import jenkins.model.Jenkins;
 import hudson.model.Item;
 import hudson.model.ItemGroup;
 import hudson.model.Items;
+import hudson.security.ACL;
 
 /**
  * Receives notifications about CRUD operations of {@link Item}.
@@ -151,26 +151,45 @@ public static ExtensionList<ItemListener> all() {
         return ExtensionList.lookup(ItemListener.class);
     }
 
-    public static void fireOnCopied(Item src, Item result) {
-        for (ItemListener l : all())
-            l.onCopied(src,result);
+    public static void fireOnCopied(final Item src, final Item result) {
+        ACL.impersonate(ACL.SYSTEM, new Runnable() {
+            @Override public void run() {
+                for (ItemListener l : all()) {
+                    l.onCopied(src, result);
+                }
+            }
+        });
     }
 
-    public static void fireOnCreated(Item item) {
-        for (ItemListener l : all())
-            l.onCreated(item);
+    public static void fireOnCreated(final Item item) {
+        ACL.impersonate(ACL.SYSTEM, new Runnable() {
+            @Override public void run() {
+                for (ItemListener l : all()) {
+                    l.onCreated(item);
+                }
+            }
+        });
     }
 
-    public static void fireOnUpdated(Item item) {
-        for (ItemListener l : all())
-            l.onUpdated(item);
+    public static void fireOnUpdated(final Item item) {
+        ACL.impersonate(ACL.SYSTEM, new Runnable() {
+            @Override public void run() {
+                for (ItemListener l : all()) {
+                    l.onUpdated(item);
+                }
+            }
+        });
     }
 
     /** @since 1.548 */
-    public static void fireOnDeleted(Item item) {
-        for (ItemListener l : all()) {
-            l.onDeleted(item);
-        }
+    public static void fireOnDeleted(final Item item) {
+        ACL.impersonate(ACL.SYSTEM, new Runnable() {
+            @Override public void run() {
+                for (ItemListener l : all()) {
+                    l.onDeleted(item);
+                }
+            }
+        });
     }
 
     /**
@@ -179,7 +198,14 @@ public static void fireOnDeleted(Item item) {
      * @param oldFullName the previous {@link Item#getFullName}
      * @since 1.548
      */
-    public static void fireLocationChange(Item rootItem, String oldFullName) {
+    public static void fireLocationChange(final Item rootItem, final String oldFullName) {
+        ACL.impersonate(ACL.SYSTEM, new Runnable() {
+            @Override public void run() {
+                doFireLocationChange(rootItem, oldFullName);
+            }
+        });
+    }
+    private static void doFireLocationChange(Item rootItem, String oldFullName) {
         String prefix = rootItem.getParent().getFullName();
         if (!prefix.isEmpty()) {
             prefix += '/';

diff --git a/test/src/test/java/hudson/model/ListViewTest.java b/test/src/test/java/hudson/model/ListViewTest.java
@@ -31,17 +31,23 @@
 import hudson.matrix.AxisList;
 import hudson.matrix.MatrixProject;
 import hudson.matrix.TextAxis;
+import hudson.security.ACL;
+import hudson.security.AuthorizationStrategy;
+import hudson.security.Permission;
 
 import java.io.IOException;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
+import org.acegisecurity.Authentication;
 
 import static org.junit.Assert.*;
 
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.Bug;
+import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.JenkinsRule.WebClient;
 import org.jvnet.hudson.test.MockFolder;
@@ -205,4 +211,40 @@ private void checkLinkFromItemExistsAndIsValid(Item item, ItemGroup ig, Item top
         assertFalse(view.contains(job));
         assertFalse(view.jobNamesContains(job));
     }
+
+    @Issue("JENKINS-22769")
+    @Test public void renameJobInViewYouCannotSee() throws Exception {
+        j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
+        j.jenkins.setAuthorizationStrategy(new AllButViewsAuthorizationStrategy());
+        final FreeStyleProject p = j.createFreeStyleProject("p1");
+        ListView v = new ListView("v", j.jenkins);
+        v.add(p);
+        j.jenkins.addView(v);
+        ACL.impersonate(User.get("alice").impersonate(), new Runnable() {
+            @Override public void run() {
+                try {
+                    p.renameTo("p2");
+                } catch (IOException x) {
+                    throw new RuntimeException(x);
+                }
+            }
+        });
+        assertEquals(Collections.singletonList(p), v.getItems());
+    }
+    private static class AllButViewsAuthorizationStrategy extends AuthorizationStrategy {
+        @Override public ACL getRootACL() {
+            return UNSECURED.getRootACL();
+        }
+        @Override public Collection<String> getGroups() {
+            return Collections.emptyList();
+        }
+        @Override public ACL getACL(View item) {
+            return new ACL() {
+                @Override public boolean hasPermission(Authentication a, Permission permission) {
+                    return a.equals(SYSTEM);
+                }
+            };
+        }
+    }
+
 }