Skip to content

Commit

Permalink
[SECURITY-2835]
Browse files Browse the repository at this point in the history
  • Loading branch information
@rsandell
rsandell committed Nov 22, 2023
1 parent c1102a5 commit 8b88d59
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 72 deletions.
11 changes: 5 additions & 6 deletions src/main/java/com/google/jenkins/plugins/computeengine/AutofilledNetworkConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.logging.Level;
import java.util.logging.Logger;
import jenkins.model.Jenkins;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.QueryParameter;
Expand Down Expand Up @@ -56,7 +57,7 @@ public ListBoxModel doFillNetworkItems(
@AncestorInPath Jenkins context,
@QueryParameter("projectId") @RelativePath("../..") final String projectId,
@QueryParameter("credentialsId") @RelativePath("../..") final String credentialsId) {
checkPermissions();
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
ListBoxModel items = new ListBoxModel();
items.add("");

Expand All @@ -78,8 +79,7 @@ public ListBoxModel doFillNetworkItems(
}

public FormValidation doCheckNetwork(@QueryParameter String value) {
checkPermissions();
if (value.equals("")) {
if (StringUtils.isEmpty(value)) {
return FormValidation.error("Please select a network...");
}
return FormValidation.ok();
Expand All @@ -91,8 +91,8 @@ public ListBoxModel doFillSubnetworkItems(
@QueryParameter("region") @RelativePath("..") final String region,
@QueryParameter("projectId") @RelativePath("../..") final String projectId,
@QueryParameter("credentialsId") @RelativePath("../..") final String credentialsId) {
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
ListBoxModel items = new ListBoxModel();
checkPermissions();

if (Strings.isNullOrEmpty(region)) {
return items;
Expand All @@ -105,7 +105,7 @@ public ListBoxModel doFillSubnetworkItems(
if (subnetworks.size() <= 1) {
items.add(new ListBoxModel.Option("", "", false));
}
if (subnetworks.size() == 0) {
if (subnetworks.isEmpty()) {
items.add(new ListBoxModel.Option("default", "default", true));
return items;
}
Expand All @@ -124,7 +124,6 @@ public ListBoxModel doFillSubnetworkItems(
}

public FormValidation doCheckSubnetwork(@QueryParameter String value) {
checkPermissions();
if (value.isEmpty()) {
return FormValidation.error("Please select a subnetwork...");
}
Expand Down
36 changes: 17 additions & 19 deletions src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@
import com.google.common.collect.ImmutableMap;
import com.google.jenkins.plugins.computeengine.client.ClientUtil;
import com.google.jenkins.plugins.credentials.oauth.GoogleOAuth2Credentials;
import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.Extension;
import hudson.model.Computer;
import hudson.model.Descriptor;
import hudson.model.Item;
import hudson.model.Job;
import hudson.model.Label;
import hudson.model.Node;
import hudson.model.TaskListener;
import hudson.security.ACL;
import hudson.security.AccessControlled;
import hudson.security.Permission;
import hudson.slaves.AbstractCloudImpl;
import hudson.slaves.Cloud;
Expand Down Expand Up @@ -423,7 +423,7 @@ public InstanceConfiguration getInstanceConfigurationByDescription(String descri
@RequirePOST
public HttpResponse doProvision(@QueryParameter String configuration)
throws ServletException, IOException {
checkPermissions(PROVISION);
checkPermissions(this, PROVISION);
if (configuration == null) {
throw HttpResponses.error(SC_BAD_REQUEST, "The 'configuration' query parameter is missing");
}
Expand All @@ -442,19 +442,21 @@ public HttpResponse doProvision(@QueryParameter String configuration)
/**
* Ensures the executing user has the specified permissions.
*
* @param permissions The list of permissions to be checked. If empty, defaults to Job.CONFIGURE.
* @param context The context on which to check the permissions, if <code>null</code> defaults
* to {@link Jenkins} And if {@link Jenkins} is also <code>null</code> then no permission
* checks will be performed.
* @param permissions The list of permissions to be checked. If empty, defaults to Jenkins.ADMINISTER.
* @throws AccessDeniedException If the user lacks the proper permissions.
* @see Jenkins#get()
*/
static void checkPermissions(Permission... permissions) {
Jenkins jenkins = Jenkins.getInstanceOrNull();
if (jenkins != null) {
if (permissions.length > 0) {
for (Permission permission : permissions) {
jenkins.checkPermission(permission);
}
} else {
jenkins.checkPermission(Job.CONFIGURE);
static void checkPermissions(@NonNull AccessControlled context, Permission... permissions) {

if (permissions.length > 0) {
for (Permission permission : permissions) {
context.checkPermission(permission);
}
} else {
context.checkPermission(Jenkins.ADMINISTER);
}
}

Expand All @@ -468,7 +470,6 @@ public String getDisplayName() {
}

public FormValidation doCheckProjectId(@QueryParameter String value) {
checkPermissions();
if (value == null || value.isEmpty()) {
return FormValidation.error("Project ID is required");
}
Expand All @@ -477,10 +478,7 @@ public FormValidation doCheckProjectId(@QueryParameter String value) {

public ListBoxModel doFillCredentialsIdItems(
@AncestorInPath Jenkins context, @QueryParameter String value) {
checkPermissions();
if (context == null || !context.hasPermission(Item.CONFIGURE)) {
return new StandardListBoxModel();
}
checkPermissions(Jenkins.getInstanceOrNull(), Jenkins.ADMINISTER);

List<DomainRequirement> domainRequirements = new ArrayList<DomainRequirement>();
return new StandardListBoxModel()
Expand All @@ -496,7 +494,7 @@ public FormValidation doCheckCredentialsId(
@AncestorInPath Jenkins context,
@QueryParameter("projectId") String projectId,
@QueryParameter String value) {
checkPermissions();
checkPermissions(Jenkins.getInstanceOrNull(), Jenkins.ADMINISTER);
if (value.isEmpty()) return FormValidation.error("No credential selected");

if (projectId.isEmpty())
Expand Down
39 changes: 14 additions & 25 deletions src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,6 @@ public String getHelpFile(String fieldName) {
}

public FormValidation doCheckNetworkTags(@QueryParameter String value) {
checkPermissions();
if (value == null || value.isEmpty()) {
return FormValidation.ok();
}
Expand All @@ -657,7 +656,6 @@ public FormValidation doCheckNetworkTags(@QueryParameter String value) {
}

public FormValidation doCheckNamePrefix(@QueryParameter String value) {
checkPermissions();
if (value == null || value.isEmpty()) {
return FormValidation.error("A prefix is required");
}
Expand All @@ -675,7 +673,6 @@ public FormValidation doCheckNamePrefix(@QueryParameter String value) {
}

public FormValidation doCheckDescription(@QueryParameter String value) {
checkPermissions();
if (value == null || value.isEmpty()) {
return FormValidation.error("A description is required");
}
Expand All @@ -686,7 +683,7 @@ public ListBoxModel doFillRegionItems(
@AncestorInPath Jenkins context,
@QueryParameter("projectId") @RelativePath("..") final String projectId,
@QueryParameter("credentialsId") @RelativePath("..") final String credentialsId) {
checkPermissions();
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
ListBoxModel items = new ListBoxModel();
items.add("");
try {
Expand All @@ -708,7 +705,7 @@ public ListBoxModel doFillTemplateItems(
@AncestorInPath Jenkins context,
@QueryParameter("projectId") @RelativePath("..") final String projectId,
@QueryParameter("credentialsId") @RelativePath("..") final String credentialsId) {
checkPermissions();
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
ListBoxModel items = new ListBoxModel();
items.add("");
try {
Expand All @@ -727,8 +724,7 @@ public ListBoxModel doFillTemplateItems(
}

public FormValidation doCheckRegion(@QueryParameter String value) {
checkPermissions();
if (value.equals("")) {
if (StringUtils.isEmpty(value)) {
return FormValidation.error("Please select a region...");
}
return FormValidation.ok();
Expand All @@ -739,7 +735,7 @@ public ListBoxModel doFillZoneItems(
@QueryParameter("projectId") @RelativePath("..") final String projectId,
@QueryParameter("region") final String region,
@QueryParameter("credentialsId") @RelativePath("..") final String credentialsId) {
checkPermissions();
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
ListBoxModel items = new ListBoxModel();
items.add("");
try {
Expand All @@ -761,8 +757,7 @@ public ListBoxModel doFillZoneItems(
}

public FormValidation doCheckZone(@QueryParameter String value) {
checkPermissions();
if (value.equals("")) {
if (StringUtils.isEmpty(value)) {
return FormValidation.error("Please select a zone...");
}
return FormValidation.ok();
Expand All @@ -773,7 +768,7 @@ public ListBoxModel doFillMachineTypeItems(
@QueryParameter("projectId") @RelativePath("..") final String projectId,
@QueryParameter("zone") final String zone,
@QueryParameter("credentialsId") @RelativePath("..") final String credentialsId) {
checkPermissions();
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
ListBoxModel items = new ListBoxModel();
items.add("");
try {
Expand All @@ -795,8 +790,7 @@ public ListBoxModel doFillMachineTypeItems(
}

public FormValidation doCheckMachineType(@QueryParameter String value) {
checkPermissions();
if (value.equals("")) {
if (StringUtils.isEmpty(value)) {
return FormValidation.error("Please select a machine type...");
}
return FormValidation.ok();
Expand All @@ -807,7 +801,7 @@ public ListBoxModel doFillMinCpuPlatformItems(
@QueryParameter("projectId") @RelativePath("..") final String projectId,
@QueryParameter("zone") final String zone,
@QueryParameter("credentialsId") @RelativePath("..") final String credentialsId) {
checkPermissions();
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
ListBoxModel items = new ListBoxModel();
items.add("");
try {
Expand All @@ -833,7 +827,7 @@ public ListBoxModel doFillBootDiskTypeItems(
@QueryParameter("projectId") @RelativePath("..") final String projectId,
@QueryParameter("zone") String zone,
@QueryParameter("credentialsId") @RelativePath("..") final String credentialsId) {
checkPermissions();
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
ListBoxModel items = new ListBoxModel();
try {
ComputeClient compute = computeClient(context, credentialsId);
Expand All @@ -856,7 +850,7 @@ public ListBoxModel doFillBootDiskTypeItems(
public ListBoxModel doFillBootDiskSourceImageProjectItems(
@AncestorInPath Jenkins context,
@QueryParameter("projectId") @RelativePath("..") final String projectId) {
checkPermissions();
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
ListBoxModel items = new ListBoxModel();
items.add("");
items.add(projectId);
Expand All @@ -867,8 +861,7 @@ public ListBoxModel doFillBootDiskSourceImageProjectItems(
}

public FormValidation doCheckBootDiskSourceImageProject(@QueryParameter String value) {
checkPermissions();
if (value.equals("")) {
if (StringUtils.isEmpty(value)) {
return FormValidation.warning("Please select source image project...");
}
return FormValidation.ok();
Expand All @@ -878,7 +871,7 @@ public ListBoxModel doFillBootDiskSourceImageNameItems(
@AncestorInPath Jenkins context,
@QueryParameter("bootDiskSourceImageProject") final String projectId,
@QueryParameter("credentialsId") @RelativePath("..") final String credentialsId) {
checkPermissions();
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
ListBoxModel items = new ListBoxModel();
items.add("");
try {
Expand All @@ -899,8 +892,7 @@ public ListBoxModel doFillBootDiskSourceImageNameItems(
}

public FormValidation doCheckBootDiskSourceImageName(@QueryParameter String value) {
checkPermissions();
if (value.equals("")) {
if (StringUtils.isEmpty(value)) {
return FormValidation.warning("Please select source image...");
}
return FormValidation.ok();
Expand All @@ -912,7 +904,7 @@ public FormValidation doCheckBootDiskSizeGbStr(
@QueryParameter("bootDiskSourceImageProject") final String projectId,
@QueryParameter("bootDiskSourceImageName") final String imageName,
@QueryParameter("credentialsId") @RelativePath("..") final String credentialsId) {
checkPermissions();
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
if (Strings.isNullOrEmpty(credentialsId)
|| Strings.isNullOrEmpty(projectId)
|| Strings.isNullOrEmpty(imageName)) return FormValidation.ok();
Expand All @@ -936,7 +928,6 @@ public FormValidation doCheckBootDiskSizeGbStr(

public FormValidation doCheckLabelString(
@QueryParameter String value, @QueryParameter Node.Mode mode) {
checkPermissions();
if (mode == Node.Mode.EXCLUSIVE && (value == null || value.trim().isEmpty())) {
return FormValidation.warning(
"You may want to assign labels to this node;"
Expand All @@ -949,7 +940,6 @@ public FormValidation doCheckCreateSnapshot(
@AncestorInPath Jenkins context,
@QueryParameter boolean value,
@QueryParameter("oneShot") boolean oneShot) {
checkPermissions();
if (!oneShot && value) {
return FormValidation.error(Messages.InstanceConfiguration_SnapshotConfigError());
}
Expand All @@ -960,7 +950,6 @@ public FormValidation doCheckNumExecutorsStr(
@AncestorInPath Jenkins context,
@QueryParameter String value,
@QueryParameter("oneShot") boolean oneShot) {
checkPermissions();
int numExecutors = intOrDefault(value, DEFAULT_NUM_EXECUTORS);
if (numExecutors < 1) {
return FormValidation.error(
Expand Down
7 changes: 4 additions & 3 deletions src/main/java/com/google/jenkins/plugins/computeengine/SharedVpcNetworkConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import hudson.Extension;
import hudson.RelativePath;
import hudson.util.FormValidation;
import jenkins.model.Jenkins;
import lombok.Getter;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.QueryParameter;
Expand Down Expand Up @@ -47,15 +48,15 @@ public String getDisplayName() {
}

public FormValidation doCheckProjectId(@QueryParameter String value) {
checkPermissions();
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
if (Strings.isNullOrEmpty(value)) {
return FormValidation.error("Project ID required");
}
return FormValidation.ok();
}

public FormValidation doCheckSubnetworkName(@QueryParameter String value) {
checkPermissions();
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
if (Strings.isNullOrEmpty(value)) {
return FormValidation.error("Subnetwork name required");
}
Expand All @@ -69,7 +70,7 @@ public FormValidation doCheckSubnetworkName(@QueryParameter String value) {
public FormValidation doCheckRegion(
@QueryParameter String value,
@QueryParameter("region") @RelativePath("..") final String region) {
checkPermissions();
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
if (Strings.isNullOrEmpty(region)
|| Strings.isNullOrEmpty(value)
|| !region.endsWith(value)) {
Expand Down
8 changes: 2 additions & 6 deletions src/main/java/com/google/jenkins/plugins/computeengine/SshConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import hudson.Extension;
import hudson.model.Describable;
import hudson.model.Descriptor;
import hudson.model.Item;
import hudson.security.ACL;
import hudson.util.FormValidation;
import hudson.util.ListBoxModel;
Expand Down Expand Up @@ -95,10 +94,7 @@ public static final class DescriptorImpl extends Descriptor<SshConfiguration> {

public ListBoxModel doFillCustomPrivateKeyCredentialsIdItems(
@AncestorInPath Jenkins context, @QueryParameter String customPrivateKeyCredentialsId) {
checkPermissions();
if (context == null || !context.hasPermission(Item.CONFIGURE)) {
return new StandardListBoxModel();
}
checkPermissions(context, Jenkins.ADMINISTER);

StandardListBoxModel result = new StandardListBoxModel();

Expand All @@ -116,7 +112,7 @@ public FormValidation doCheckCustomPrivateKeyCredentialsId(
@QueryParameter String value,
@QueryParameter("customPrivateKeyCredentialsId") String customPrivateKeyCredentialsId)
throws IOException {
checkPermissions();
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);

if (Strings.isNullOrEmpty(value) && Strings.isNullOrEmpty(customPrivateKeyCredentialsId)) {
return FormValidation.error("An SSH private key credential is required");
Expand Down

0 comments on commit 8b88d59

Please sign in to comment.