Skip to content

Commit

Permalink
[SECURITY-201] - UX: Also report information about missing BUILD perm…
Browse files Browse the repository at this point in the history
…issions in the build log
  • Loading branch information
oleg-nenashev committed Jun 9, 2017
1 parent 2c8cd7b commit 89a6148
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ListMultimap;
import hudson.model.TaskListener;

import java.io.IOException;
import java.util.Collection;
Expand Down Expand Up @@ -69,7 +70,7 @@ public ListMultimap<Job, Future<Run>> perform3(AbstractBuild<?, ?> build, Launch
}

@Override
protected Future schedule(AbstractBuild<?, ?> build, Job project, List<Action> list) throws InterruptedException, IOException {
protected Future schedule(AbstractBuild<?, ?> build, Job project, List<Action> list, TaskListener listener) throws InterruptedException, IOException {
if (block!=null) {
while (true) {
// add DifferentiatingAction to make sure this doesn't get merged with something else,
Expand All @@ -78,15 +79,15 @@ protected Future schedule(AbstractBuild<?, ?> build, Job project, List<Action> l

// if we fail to add the item to the queue, wait and retry.
// it also means we have to force quiet period = 0, or else it'll never leave the queue
Future f = schedule(build, project, 0, list);
Future f = schedule(build, project, 0, list, listener);
// When a project is disabled or the configuration is not yet saved f will always be null and we're caught in a loop, therefore we need to check for it
if (f!=null || (f==null && !canBeScheduled(project))){
return f;
}
Thread.sleep(1000);
}
} else {
return super.schedule(build,project,list);
return super.schedule(build,project,list,listener);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ public List<Future<AbstractBuild>> perform(AbstractBuild<?, ?> build, Launcher l
for (Job project : getJobs(build.getRootBuild().getProject().getParent(), env)) {
List<Action> list = getBuildActions(actions, project);

futures.add(schedule(build, project, list));
futures.add(schedule(build, project, list, listener));
}
}

Expand Down Expand Up @@ -443,7 +443,7 @@ public ListMultimap<Job, Future<Run>> perform3(AbstractBuild<?, ?> build, Launch
for (Job project : getJobs(build.getRootBuild().getProject().getParent(), env)) {
List<Action> list = getBuildActions(actions, project);

futures.put(project, schedule(build, project, list));
futures.put(project, schedule(build, project, list, listener));
}
}
return futures;
Expand Down Expand Up @@ -511,8 +511,18 @@ protected Cause createUpstreamCause(Run<?, ?> build) {
return new UpstreamCause(build);
}

/**
* @deprecated Use {@link #schedule(hudson.model.AbstractBuild, hudson.model.Job, java.util.List, hudson.model.TaskListener)}
*/
@CheckForNull
@Deprecated
protected Future schedule(AbstractBuild<?, ?> build, final Job project, int quietPeriod, List<Action> list) throws InterruptedException, IOException {
return schedule(build, project, quietPeriod, list, TaskListener.NULL);
}

@CheckForNull
protected Future schedule(@Nonnull AbstractBuild<?, ?> build, @Nonnull final Job project, int quietPeriod,
@Nonnull List<Action> list, @Nonnull TaskListener listener) throws InterruptedException, IOException {
// TODO Once it's in core (since 1.621) and LTS is out, switch to use new ParameterizedJobMixIn convenience method
// From https://github.com/jenkinsci/jenkins/pull/1771
Cause cause = createUpstreamCause(build);
Expand All @@ -533,7 +543,7 @@ protected Future schedule(AbstractBuild<?, ?> build, final Job project, int quie
// We check the user permissions.
// QueueItemAuthenticator should provide the user if it is configured correctly.
//TODO: It would be also great to print it to the build log, but there is no TaskListener
if (!canTriggerProject(build, project, null)) {
if (!canTriggerProject(build, project, listener)) {
return null;
}

Expand All @@ -548,29 +558,19 @@ protected Future schedule(AbstractBuild<?, ?> build, final Job project, int quie
* Checks if the build can trigger a project.
* @param build Build, which is about to trigger the project
* @param job Job to be triggered
* @param taskListener Optional task listener
* @param taskListener Task listener
* @return {@code true} if the project can be scheduled.
* {@code false} if there is a lack of permissions, details will be printed to the logs then.
*/
/*package*/ static boolean canTriggerProject(@Nonnull AbstractBuild<?, ?> build,
@Nonnull final Job job, @CheckForNull TaskListener taskListener) {
@Nonnull final Job job, @Nonnull TaskListener taskListener) {
if (!job.hasPermission(Item.BUILD)) {
//TODO: It would be also great to print it to the build log, but there is no TaskListener
String message = null;
if (LOGGER.isLoggable(Level.WARNING) || taskListener != null) {
message = String.format("Cannot schedule the build of %s from %s. "
String message = String.format("Cannot schedule the build of %s from %s. "
+ "The authenticated build user %s has no Item.BUILD permission",
job, build, Jenkins.getAuthentication());
}

if (message != null) {
LOGGER.log(Level.WARNING, message);
}

if (taskListener != null) {
taskListener.error(message);
}

LOGGER.log(Level.WARNING, message);
taskListener.error(message);
return false;
}
return true;
Expand All @@ -590,13 +590,20 @@ protected boolean canBeScheduled(@Nonnull Job<?, ?> job) {
return job.hasPermission(Item.BUILD);
}

/**
* @deprecated Use {@link #schedule(hudson.model.AbstractBuild, hudson.model.Job, int, java.util.List, hudson.model.TaskListener)}
*/
@Deprecated
protected Future schedule(AbstractBuild<?, ?> build, Job project, List<Action> list) throws InterruptedException, IOException {
return schedule(build, project, list, TaskListener.NULL);
}

protected Future schedule(@Nonnull AbstractBuild<?, ?> build, @Nonnull Job project, @Nonnull List<Action> list, @Nonnull TaskListener listener) throws InterruptedException, IOException {
if (project instanceof ParameterizedJobMixIn.ParameterizedJob) {
return schedule(build, project, ((ParameterizedJobMixIn.ParameterizedJob) project).getQuietPeriod(), list);
return schedule(build, project, ((ParameterizedJobMixIn.ParameterizedJob) project).getQuietPeriod(), list, listener);
} else {
return schedule(build, project, 0, list);
return schedule(build, project, 0, list, listener);
}

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ public void shouldBeUnableToTriggerWithoutPermissions(boolean useBuildStep) thro
// Assert the subproject1 has not been built
assertTrue("The subproject1 has been triggered, but it should not happen due to the build permissions",
subproject1.getBuilds().isEmpty());
r.assertLogContains("has no Item.BUILD permission", build);

if (!useBuildStep) {
// Non-blocking, we have to check the status
Expand Down

0 comments on commit 89a6148

Please sign in to comment.