diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/AutofilledNetworkConfiguration.java b/src/main/java/com/google/jenkins/plugins/computeengine/AutofilledNetworkConfiguration.java
index b5e76d3d..b2040208 100644
--- a/src/main/java/com/google/jenkins/plugins/computeengine/AutofilledNetworkConfiguration.java
+++ b/src/main/java/com/google/jenkins/plugins/computeengine/AutofilledNetworkConfiguration.java
@@ -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;
@@ -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("");
@@ -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();
@@ -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;
@@ -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;
}
@@ -124,7 +124,6 @@ public ListBoxModel doFillSubnetworkItems(
}
public FormValidation doCheckSubnetwork(@QueryParameter String value) {
- checkPermissions();
if (value.isEmpty()) {
return FormValidation.error("Please select a subnetwork...");
}
diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java
index 860d8854..e46187ff 100644
--- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java
+++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java
@@ -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;
@@ -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");
}
@@ -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 null
defaults
+ * to {@link Jenkins} And if {@link Jenkins} is also null
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);
}
}
@@ -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");
}
@@ -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 domainRequirements = new ArrayList();
return new StandardListBoxModel()
@@ -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())
diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java b/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java
index ea4d0d57..291bbc87 100644
--- a/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java
+++ b/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java
@@ -640,7 +640,6 @@ public String getHelpFile(String fieldName) {
}
public FormValidation doCheckNetworkTags(@QueryParameter String value) {
- checkPermissions();
if (value == null || value.isEmpty()) {
return FormValidation.ok();
}
@@ -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");
}
@@ -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");
}
@@ -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 {
@@ -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 {
@@ -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();
@@ -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 {
@@ -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();
@@ -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 {
@@ -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();
@@ -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 {
@@ -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);
@@ -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);
@@ -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();
@@ -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 {
@@ -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();
@@ -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();
@@ -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;"
@@ -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());
}
@@ -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(
diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/SharedVpcNetworkConfiguration.java b/src/main/java/com/google/jenkins/plugins/computeengine/SharedVpcNetworkConfiguration.java
index 337373a8..e833a382 100644
--- a/src/main/java/com/google/jenkins/plugins/computeengine/SharedVpcNetworkConfiguration.java
+++ b/src/main/java/com/google/jenkins/plugins/computeengine/SharedVpcNetworkConfiguration.java
@@ -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;
@@ -47,7 +48,7 @@ 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");
}
@@ -55,7 +56,7 @@ public FormValidation doCheckProjectId(@QueryParameter String value) {
}
public FormValidation doCheckSubnetworkName(@QueryParameter String value) {
- checkPermissions();
+ checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
if (Strings.isNullOrEmpty(value)) {
return FormValidation.error("Subnetwork name required");
}
@@ -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)) {
diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/SshConfiguration.java b/src/main/java/com/google/jenkins/plugins/computeengine/SshConfiguration.java
index 86d0695a..5f0718b4 100644
--- a/src/main/java/com/google/jenkins/plugins/computeengine/SshConfiguration.java
+++ b/src/main/java/com/google/jenkins/plugins/computeengine/SshConfiguration.java
@@ -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;
@@ -95,10 +94,7 @@ public static final class DescriptorImpl extends Descriptor {
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();
@@ -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");
diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/WindowsConfiguration.java b/src/main/java/com/google/jenkins/plugins/computeengine/WindowsConfiguration.java
index d0bf694a..59ff50d4 100644
--- a/src/main/java/com/google/jenkins/plugins/computeengine/WindowsConfiguration.java
+++ b/src/main/java/com/google/jenkins/plugins/computeengine/WindowsConfiguration.java
@@ -28,7 +28,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;
@@ -111,10 +110,7 @@ public Descriptor getDescriptor() {
@Extension
public static final class DescriptorImpl extends Descriptor {
public ListBoxModel doFillPasswordCredentialsIdItems(@AncestorInPath Jenkins context) {
- checkPermissions();
- if (context == null || !context.hasPermission(Item.CONFIGURE)) {
- return new StandardListBoxModel();
- }
+ checkPermissions(context, Jenkins.ADMINISTER);
return new StandardListBoxModel()
.withEmptySelection()
.withMatching(
@@ -127,10 +123,7 @@ public ListBoxModel doFillPasswordCredentialsIdItems(@AncestorInPath Jenkins con
}
public ListBoxModel doFillPrivateKeyCredentialsIdItems(@AncestorInPath Jenkins context) {
- checkPermissions();
- if (context == null || !context.hasPermission(Item.CONFIGURE)) {
- return new StandardUsernameListBoxModel();
- }
+ checkPermissions(context, Jenkins.ADMINISTER);
return new StandardUsernameListBoxModel()
.withEmptySelection()
.withMatching(
@@ -142,7 +135,6 @@ public ListBoxModel doFillPrivateKeyCredentialsIdItems(@AncestorInPath Jenkins c
public FormValidation doCheckPrivateKeyCredentialsId(
@QueryParameter String value,
@QueryParameter("passwordCredentialsId") String passwordCredentialsId) {
- checkPermissions();
if (Strings.isNullOrEmpty(value) && Strings.isNullOrEmpty(passwordCredentialsId)) {
return FormValidation.error("A password or private key credential is required");
}
diff --git a/src/test/java/com/google/jenkins/plugins/computeengine/SharedVpcNetworkConfigurationTest.java b/src/test/java/com/google/jenkins/plugins/computeengine/SharedVpcNetworkConfigurationTest.java
index 0744a4f9..2d9ecaca 100644
--- a/src/test/java/com/google/jenkins/plugins/computeengine/SharedVpcNetworkConfigurationTest.java
+++ b/src/test/java/com/google/jenkins/plugins/computeengine/SharedVpcNetworkConfigurationTest.java
@@ -20,8 +20,12 @@
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue;
+import hudson.ExtensionList;
import hudson.util.FormValidation;
+import org.junit.Rule;
import org.junit.Test;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.WithoutJenkins;
public class SharedVpcNetworkConfigurationTest {
public final String PROJECT_ID = "test-project";
@@ -29,7 +33,10 @@ public class SharedVpcNetworkConfigurationTest {
public final String BAD_SUBNETWORK_NAME = "projects/project/subnetworks/test-subnetwork";
public final String REGION = "test-region";
+ @Rule public JenkinsRule r = new JenkinsRule();
+
@Test
+ @WithoutJenkins
public void construction() {
// Empty constructor
SharedVpcNetworkConfiguration anc =
@@ -44,7 +51,7 @@ public void construction() {
@Test
public void descriptorSubnetwork() {
SharedVpcNetworkConfiguration.DescriptorImpl d =
- new SharedVpcNetworkConfiguration.DescriptorImpl();
+ ExtensionList.lookupSingleton(SharedVpcNetworkConfiguration.DescriptorImpl.class);
// Subnetwork with slash returns an error
FormValidation fv = d.doCheckSubnetworkName(BAD_SUBNETWORK_NAME);
@@ -66,7 +73,7 @@ public void descriptorSubnetwork() {
@Test
public void descriptorProjectId() {
SharedVpcNetworkConfiguration.DescriptorImpl d =
- new SharedVpcNetworkConfiguration.DescriptorImpl();
+ ExtensionList.lookupSingleton(SharedVpcNetworkConfiguration.DescriptorImpl.class);
// Empty project returns an error
FormValidation fv = d.doCheckProjectId("");
@@ -84,7 +91,7 @@ public void descriptorProjectId() {
@Test
public void descriptorRegion() {
SharedVpcNetworkConfiguration.DescriptorImpl d =
- new SharedVpcNetworkConfiguration.DescriptorImpl();
+ ExtensionList.lookupSingleton(SharedVpcNetworkConfiguration.DescriptorImpl.class);
// All empty params returns an error
FormValidation fv = d.doCheckRegion("region-textbox", "");