[FIXED JENKINS-25400] Rework fix of JENKINS-22769 (c04cdcd) to put th…

…e burden on each listener to impersonate ACL.SYSTEM if it needs to.
jenkinsci · Nov 4, 2014 · a6a3d5e · a6a3d5e
1 parent b486e46
commit a6a3d5e
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 34 deletions.
diff --git a/changelog.html b/changelog.html
@@ -55,6 +55,9 @@
 <!-- Record your changes in the trunk here. -->
 <div id="trunk" style="display:none"><!--=TRUNK-BEGIN=-->
 <ul class=image>
+  <li class=bug>
+    Some plugins broken since 1.584 if they expected certain events to be fired under a specific user ID.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-25400">issue 25400</a>)
   <li class=bug>
     Fixed various real or potential resource leaks.
     (<a href="https://github.com/jenkinsci/jenkins/pull/1434">pull request 1434</a>)  

diff --git a/core/src/main/java/hudson/model/Fingerprint.java b/core/src/main/java/hudson/model/Fingerprint.java
@@ -39,6 +39,7 @@
 import hudson.Extension;
 import hudson.model.listeners.ItemListener;
 import hudson.model.listeners.SaveableListener;
+import hudson.security.ACL;
 import hudson.util.AtomicFileWriter;
 import hudson.util.HexBinaryConverter;
 import hudson.util.Iterators;
@@ -729,7 +730,14 @@ public Object unmarshal(HierarchicalStreamReader reader, final UnmarshallingCont
     @Extension
     public static final class ProjectRenameListener extends ItemListener {
         @Override
-        public void onLocationChanged(Item item, String oldName, String newName) {
+        public void onLocationChanged(final Item item, final String oldName, final String newName) {
+            ACL.impersonate(ACL.SYSTEM, new Runnable() {
+                @Override public void run() {
+                    locationChanged(item, oldName, newName);
+                }
+            });
+        }
+        private void locationChanged(Item item, String oldName, String newName) {
             if (item instanceof AbstractProject) {
                 AbstractProject p = Jenkins.getInstance().getItemByFullName(newName, AbstractProject.class);
                 if (p != null) {

diff --git a/core/src/main/java/hudson/model/ListView.java b/core/src/main/java/hudson/model/ListView.java
@@ -29,6 +29,7 @@
 import hudson.diagnosis.OldDataMonitor;
 import hudson.model.Descriptor.FormException;
 import hudson.model.listeners.ItemListener;
+import hudson.security.ACL;
 import hudson.util.CaseInsensitiveComparator;
 import hudson.util.DescribableList;
 import hudson.util.FormValidation;
@@ -431,7 +432,14 @@ public static List<ListViewColumn> getDefaultColumns() {
 
     @Restricted(NoExternalUse.class)
     @Extension public static final class Listener extends ItemListener {
-        @Override public void onLocationChanged(Item item, String oldFullName, String newFullName) {
+        @Override public void onLocationChanged(final Item item, final String oldFullName, final String newFullName) {
+            ACL.impersonate(ACL.SYSTEM, new Runnable() {
+                @Override public void run() {
+                    locationChanged(item, oldFullName, newFullName);
+                }
+            });
+        }
+        private void locationChanged(Item item, String oldFullName, String newFullName) {
             final Jenkins jenkins = Jenkins.getInstance();
             for (View view: jenkins.getViews()) {
                 if (view instanceof ListView) {
@@ -469,7 +477,14 @@ private void renameViewItem(String oldFullName, String newFullName, ViewGroup vg
             }
         }
 
-        @Override public void onDeleted(Item item) {
+        @Override public void onDeleted(final Item item) {
+            ACL.impersonate(ACL.SYSTEM, new Runnable() {
+                @Override public void run() {
+                    deleted(item);
+                }
+            });
+        }
+        private void deleted(Item item) {
             final Jenkins jenkins = Jenkins.getInstance();
             for (View view: jenkins.getViews()) {
                 if (view instanceof ListView) {

diff --git a/core/src/main/java/hudson/model/listeners/ItemListener.java b/core/src/main/java/hudson/model/listeners/ItemListener.java
@@ -31,8 +31,10 @@
 import hudson.model.ItemGroup;
 import hudson.model.Items;
 import hudson.security.ACL;
+import java.util.List;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import jenkins.security.NotReallyRoleSensitiveCallable;
 
 /**
  * Receives notifications about CRUD operations of {@link Item}.
@@ -158,22 +160,13 @@ public static ExtensionList<ItemListener> all() {
     }
 
     // TODO JENKINS-21224 generalize this to a method perhaps in ExtensionList and use consistently from all listeners
-    private static void forAll(final /* java.util.function.Consumer<ItemListener> */Function<ItemListener,Void> consumer, boolean asSystem) {
-        Runnable r = new Runnable() {
-            @Override public void run() {
-                for (ItemListener l : all()) {
-                    try {
-                        consumer.apply(l);
-                    } catch (RuntimeException x) {
-                        LOGGER.log(Level.WARNING, "failed to send event to listener of " + l.getClass(), x);
-                    }
-                }
+    private static void forAll(final /* java.util.function.Consumer<ItemListener> */Function<ItemListener,Void> consumer) {
+        for (ItemListener l : all()) {
+            try {
+                consumer.apply(l);
+            } catch (RuntimeException x) {
+                LOGGER.log(Level.WARNING, "failed to send event to listener of " + l.getClass(), x);
             }
-        };
-        if (asSystem) {
-            ACL.impersonate(ACL.SYSTEM, r);
-        } else {
-            r.run();
         }
     }
 
@@ -183,7 +176,7 @@ public static void fireOnCopied(final Item src, final Item result) {
                 l.onCopied(src, result);
                 return null;
             }
-        }, true);
+        });
     }
 
     public static void fireOnCreated(final Item item) {
@@ -192,7 +185,7 @@ public static void fireOnCreated(final Item item) {
                 l.onCreated(item);
                 return null;
             }
-        }, true);
+        });
     }
 
     public static void fireOnUpdated(final Item item) {
@@ -201,7 +194,7 @@ public static void fireOnUpdated(final Item item) {
                 l.onUpdated(item);
                 return null;
             }
-        }, true);
+        });
     }
 
     /** @since 1.548 */
@@ -211,7 +204,7 @@ public static void fireOnDeleted(final Item item) {
                 l.onDeleted(item);
                 return null;
             }
-        }, true);
+        });
     }
 
     /**
@@ -221,13 +214,6 @@ public static void fireOnDeleted(final Item item) {
      * @since 1.548
      */
     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(final Item rootItem, final String oldFullName) {
         String prefix = rootItem.getParent().getFullName();
         if (!prefix.isEmpty()) {
             prefix += '/';
@@ -244,16 +230,20 @@ private static void doFireLocationChange(final Item rootItem, final String oldFu
                     l.onRenamed(rootItem, oldName, newName);
                     return null;
                 }
-            }, false);
+            });
         }
         forAll(new Function<ItemListener, Void>() {
             @Override public Void apply(ItemListener l) {
                 l.onLocationChanged(rootItem, oldFullName, newFullName);
                 return null;
             }
-        }, false);
+        });
         if (rootItem instanceof ItemGroup) {
-            for (final Item child : Items.getAllItems((ItemGroup) rootItem, Item.class)) {
+            for (final Item child : ACL.impersonate(ACL.SYSTEM, new NotReallyRoleSensitiveCallable<List<Item>,RuntimeException>() {
+                @Override public List<Item> call() {
+                    return Items.getAllItems((ItemGroup) rootItem, Item.class);
+                }
+            })) {
                 final String childNew = child.getFullName();
                 assert childNew.startsWith(newFullName);
                 assert childNew.charAt(newFullName.length()) == '/';
@@ -263,7 +253,7 @@ private static void doFireLocationChange(final Item rootItem, final String oldFu
                         l.onLocationChanged(child, childOld, childNew);
                         return null;
                     }
-                }, false);
+                });
             }
         }
     }

diff --git a/core/src/main/java/hudson/tasks/BuildTrigger.java b/core/src/main/java/hudson/tasks/BuildTrigger.java
@@ -406,7 +406,14 @@ public AutoCompletionCandidates doAutoCompleteChildProjects(@QueryParameter Stri
         @Extension
         public static class ItemListenerImpl extends ItemListener {
             @Override
-            public void onLocationChanged(Item item, String oldFullName, String newFullName) {
+            public void onLocationChanged(final Item item, final String oldFullName, final String newFullName) {
+                ACL.impersonate(ACL.SYSTEM, new Runnable() {
+                    @Override public void run() {
+                        locationChanged(item, oldFullName, newFullName);
+                    }
+                });
+            }
+            private void locationChanged(Item item, String oldFullName, String newFullName) {
                 // update BuildTrigger of other projects that point to this object.
                 // can't we generalize this?
                 for( Project<?,?> p : Jenkins.getInstance().getAllItems(Project.class) ) {