Skip to content
Permalink
Browse files

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

…ce they sometimes do ACL-checked calls.
  • Loading branch information...
jglick committed Sep 24, 2014
1 parent 253943e commit c04cdcd9f717ddcd3e8c9dbe86cb353c14ae511e
@@ -58,6 +58,9 @@
<li class=rfe>
When killing Windows processes, check its critical flag to avoid BSoD
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-24453">issue 24453</a>)
<li class=bug>
When a user could not see a view, but could delete/move/rename jobs contained in it, the view was not properly updated.
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-22769">issue 22769</a>)
</ul>
</div><!--=TRUNK-END=-->

@@ -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 void register() {
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 += '/';
@@ -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);
}
};
}
}

}

3 comments on commit c04cdcd

@olivergondza

This comment has been minimized.

Copy link
Member

replied Nov 1, 2014

Caused JENKINS-25400

@daniel-beck

This comment has been minimized.

Copy link
Member

replied Nov 1, 2014

Caused JENKINS-25400. AFAIK there's a similar issue with Create Job Advanced Plugin's "Grant creator full permission."

Wouldn't it be better to have listeners impersonate SYSTEM as needed on their own?

@olivergondza

This comment has been minimized.

Copy link
Member

replied Nov 1, 2014

Unfortunately, some listeners has been doing this for years (ComputerListener#onOnline) so changing it not to impersonate SYSTEM can be painful. Though, I like @daniel-beck's suggestion.

Please sign in to comment.
You can’t perform that action at this time.