Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-42556] Make PlaceholderTask.getFullDisplayName ignore thread authentication #34

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -48,6 +48,8 @@
import jenkins.util.Timer;
import org.acegisecurity.AccessDeniedException;
import org.acegisecurity.Authentication;
import org.acegisecurity.context.SecurityContext;
import org.acegisecurity.context.SecurityContextHolder;
import org.jenkinsci.plugins.durabletask.executors.ContinuableExecutable;
import org.jenkinsci.plugins.durabletask.executors.ContinuedTask;
import org.jenkinsci.plugins.workflow.flow.FlowExecution;
Expand Down Expand Up @@ -381,7 +383,12 @@ private Object readResolve() {
public @CheckForNull Run<?,?> runForDisplay() {
Run<?,?> r = run();
if (r == null && /* not stored prior to 1.13 */runId != null) {
return Run.fromExternalizableId(runId);
SecurityContext orig = ACL.impersonate(ACL.SYSTEM);
try {
return Run.fromExternalizableId(runId);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not to worry about showing the task to other users in the executor widget; this display code already handled that, since run() will normally return something without an access check (while the node block is up and running)—this code path is only used during ExecutorPickle rehydration when resuming a build.

} finally {
SecurityContextHolder.setContext(orig);
}
}
return r;
}
Expand Down
Expand Up @@ -24,9 +24,13 @@

package org.jenkinsci.plugins.workflow.support.pickles;

import hudson.model.Item;
import hudson.model.Label;
import hudson.model.Queue;
import hudson.model.User;
import hudson.slaves.DumbSlave;
import hudson.slaves.OfflineCause;
import jenkins.model.Jenkins;
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
Expand All @@ -38,12 +42,15 @@
import org.junit.Rule;
import org.junit.runners.model.Statement;
import org.jvnet.hudson.test.BuildWatcher;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.MockAuthorizationStrategy;
import org.jvnet.hudson.test.RestartableJenkinsRule;

public class ExecutorPickleTest {

@ClassRule public static BuildWatcher buildWatcher = new BuildWatcher();
@Rule public RestartableJenkinsRule r = new RestartableJenkinsRule();
//@Rule public LoggerRule logging = new LoggerRule().record(Queue.class, Level.FINE);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Useful with jenkinsci/jenkins#2791, and triggers further exceptions fixed in jenkinsci/jenkins#2792.


@Test public void canceledQueueItem() throws Exception {
r.addStep(new Statement() {
Expand Down Expand Up @@ -71,4 +78,33 @@ public class ExecutorPickleTest {
});
}

@Issue("JENKINS-42556")
@Test public void anonDiscover() {
r.addStep(new Statement() {
@Override public void evaluate() throws Throwable {
r.j.jenkins.setSecurityRealm(r.j.createDummySecurityRealm());
r.j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().
grant(Jenkins.ADMINISTER).everywhere().to("admin").
grant(Jenkins.READ, Item.DISCOVER).everywhere().toEveryone());
r.j.jenkins.save(); // TODO pending https://github.com/jenkinsci/jenkins/pull/2790
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DumbSlave remote = r.j.createSlave("remote", null, null);
WorkflowJob p = r.j.createProject(WorkflowJob.class, "p");
p.setDefinition(new CpsFlowDefinition("node('remote') {semaphore 'wait'}", true));
SemaphoreStep.waitForStart("wait/1", p.scheduleBuild2(0).waitForStart());
remote.toComputer().setTemporarilyOffline(true, new OfflineCause.UserCause(User.get("admin"), "hold"));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise ExecutorPickle could resume too quickly without ever even trying to print a message about its progress.

}
});
r.addStep(new Statement() {
@Override public void evaluate() throws Throwable {
SemaphoreStep.success("wait/1", null);
WorkflowJob p = r.j.jenkins.getItemByFullName("p", WorkflowJob.class);
assertFalse(p.getACL().hasPermission(Jenkins.ANONYMOUS, Item.READ));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

verifying that save worked

WorkflowRun b = p.getBuildByNumber(1);
r.j.waitForMessage(Messages.ExecutorPickle_waiting_to_resume(Messages.ExecutorStepExecution_PlaceholderTask_displayName(b.getFullDisplayName())), b);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r.j.jenkins.getNode("remote").toComputer().setTemporarilyOffline(false, null);
r.j.assertBuildStatusSuccess(r.j.waitForCompletion(b));
}
});
}

}