Skip to content

Commit 5a4dc98

Browse files
rsandellrsandell
committed
[SECURITY-2835]
1 parent b327fca commit 5a4dc98

File tree

7 files changed

+54
-72
lines changed

7 files changed

+54
-72
lines changed

‎src/main/java/com/google/jenkins/plugins/computeengine/AutofilledNetworkConfiguration.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.util.logging.Level;
3030
import java.util.logging.Logger;
3131
import jenkins.model.Jenkins;
32+
import org.apache.commons.lang.StringUtils;
3233
import org.kohsuke.stapler.AncestorInPath;
3334
import org.kohsuke.stapler.DataBoundConstructor;
3435
import org.kohsuke.stapler.QueryParameter;
@@ -56,7 +57,7 @@ public ListBoxModel doFillNetworkItems(
5657
@AncestorInPath Jenkins context,
5758
@QueryParameter("projectId") @RelativePath("../..") final String projectId,
5859
@QueryParameter("credentialsId") @RelativePath("../..") final String credentialsId) {
59-
checkPermissions();
60+
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
6061
ListBoxModel items = new ListBoxModel();
6162
items.add("");
6263

@@ -78,8 +79,7 @@ public ListBoxModel doFillNetworkItems(
7879
}
7980

8081
public FormValidation doCheckNetwork(@QueryParameter String value) {
81-
checkPermissions();
82-
if (value.equals("")) {
82+
if (StringUtils.isEmpty(value)) {
8383
return FormValidation.error("Please select a network...");
8484
}
8585
return FormValidation.ok();
@@ -91,8 +91,8 @@ public ListBoxModel doFillSubnetworkItems(
9191
@QueryParameter("region") @RelativePath("..") final String region,
9292
@QueryParameter("projectId") @RelativePath("../..") final String projectId,
9393
@QueryParameter("credentialsId") @RelativePath("../..") final String credentialsId) {
94+
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
9495
ListBoxModel items = new ListBoxModel();
95-
checkPermissions();
9696

9797
if (Strings.isNullOrEmpty(region)) {
9898
return items;
@@ -105,7 +105,7 @@ public ListBoxModel doFillSubnetworkItems(
105105
if (subnetworks.size() <= 1) {
106106
items.add(new ListBoxModel.Option("", "", false));
107107
}
108-
if (subnetworks.size() == 0) {
108+
if (subnetworks.isEmpty()) {
109109
items.add(new ListBoxModel.Option("default", "default", true));
110110
return items;
111111
}
@@ -124,7 +124,6 @@ public ListBoxModel doFillSubnetworkItems(
124124
}
125125

126126
public FormValidation doCheckSubnetwork(@QueryParameter String value) {
127-
checkPermissions();
128127
if (value.isEmpty()) {
129128
return FormValidation.error("Please select a subnetwork...");
130129
}

‎src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java‎

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,15 @@
3030
import com.google.common.collect.ImmutableMap;
3131
import com.google.jenkins.plugins.computeengine.client.ClientUtil;
3232
import com.google.jenkins.plugins.credentials.oauth.GoogleOAuth2Credentials;
33+
import edu.umd.cs.findbugs.annotations.NonNull;
3334
import hudson.Extension;
3435
import hudson.model.Computer;
3536
import hudson.model.Descriptor;
36-
import hudson.model.Item;
37-
import hudson.model.Job;
3837
import hudson.model.Label;
3938
import hudson.model.Node;
4039
import hudson.model.TaskListener;
4140
import hudson.security.ACL;
41+
import hudson.security.AccessControlled;
4242
import hudson.security.Permission;
4343
import hudson.slaves.AbstractCloudImpl;
4444
import hudson.slaves.Cloud;
@@ -423,7 +423,7 @@ public InstanceConfiguration getInstanceConfigurationByDescription(String descri
423423
@RequirePOST
424424
public HttpResponse doProvision(@QueryParameter String configuration)
425425
throws ServletException, IOException {
426-
checkPermissions(PROVISION);
426+
checkPermissions(this, PROVISION);
427427
if (configuration == null) {
428428
throw HttpResponses.error(SC_BAD_REQUEST, "The 'configuration' query parameter is missing");
429429
}
@@ -442,19 +442,21 @@ public HttpResponse doProvision(@QueryParameter String configuration)
442442
/**
443443
* Ensures the executing user has the specified permissions.
444444
*
445-
* @param permissions The list of permissions to be checked. If empty, defaults to Job.CONFIGURE.
445+
* @param context The context on which to check the permissions, if <code>null</code> defaults
446+
* to {@link Jenkins} And if {@link Jenkins} is also <code>null</code> then no permission
447+
* checks will be performed.
448+
* @param permissions The list of permissions to be checked. If empty, defaults to Jenkins.ADMINISTER.
446449
* @throws AccessDeniedException If the user lacks the proper permissions.
450+
* @see Jenkins#get()
447451
*/
448-
static void checkPermissions(Permission... permissions) {
449-
Jenkins jenkins = Jenkins.getInstanceOrNull();
450-
if (jenkins != null) {
451-
if (permissions.length > 0) {
452-
for (Permission permission : permissions) {
453-
jenkins.checkPermission(permission);
454-
}
455-
} else {
456-
jenkins.checkPermission(Job.CONFIGURE);
452+
static void checkPermissions(@NonNull AccessControlled context, Permission... permissions) {
453+
454+
if (permissions.length > 0) {
455+
for (Permission permission : permissions) {
456+
context.checkPermission(permission);
457457
}
458+
} else {
459+
context.checkPermission(Jenkins.ADMINISTER);
458460
}
459461
}
460462

@@ -468,7 +470,6 @@ public String getDisplayName() {
468470
}
469471

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

478479
public ListBoxModel doFillCredentialsIdItems(
479480
@AncestorInPath Jenkins context, @QueryParameter String value) {
480-
checkPermissions();
481-
if (context == null || !context.hasPermission(Item.CONFIGURE)) {
482-
return new StandardListBoxModel();
483-
}
481+
checkPermissions(Jenkins.getInstanceOrNull(), Jenkins.ADMINISTER);
484482

485483
List<DomainRequirement> domainRequirements = new ArrayList<DomainRequirement>();
486484
return new StandardListBoxModel()
@@ -496,7 +494,7 @@ public FormValidation doCheckCredentialsId(
496494
@AncestorInPath Jenkins context,
497495
@QueryParameter("projectId") String projectId,
498496
@QueryParameter String value) {
499-
checkPermissions();
497+
checkPermissions(Jenkins.getInstanceOrNull(), Jenkins.ADMINISTER);
500498
if (value.isEmpty()) return FormValidation.error("No credential selected");
501499

502500
if (projectId.isEmpty())

‎src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java‎

Lines changed: 14 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,6 @@ public String getHelpFile(String fieldName) {
640640
}
641641

642642
public FormValidation doCheckNetworkTags(@QueryParameter String value) {
643-
checkPermissions();
644643
if (value == null || value.isEmpty()) {
645644
return FormValidation.ok();
646645
}
@@ -657,7 +656,6 @@ public FormValidation doCheckNetworkTags(@QueryParameter String value) {
657656
}
658657

659658
public FormValidation doCheckNamePrefix(@QueryParameter String value) {
660-
checkPermissions();
661659
if (value == null || value.isEmpty()) {
662660
return FormValidation.error("A prefix is required");
663661
}
@@ -675,7 +673,6 @@ public FormValidation doCheckNamePrefix(@QueryParameter String value) {
675673
}
676674

677675
public FormValidation doCheckDescription(@QueryParameter String value) {
678-
checkPermissions();
679676
if (value == null || value.isEmpty()) {
680677
return FormValidation.error("A description is required");
681678
}
@@ -686,7 +683,7 @@ public ListBoxModel doFillRegionItems(
686683
@AncestorInPath Jenkins context,
687684
@QueryParameter("projectId") @RelativePath("..") final String projectId,
688685
@QueryParameter("credentialsId") @RelativePath("..") final String credentialsId) {
689-
checkPermissions();
686+
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
690687
ListBoxModel items = new ListBoxModel();
691688
items.add("");
692689
try {
@@ -708,7 +705,7 @@ public ListBoxModel doFillTemplateItems(
708705
@AncestorInPath Jenkins context,
709706
@QueryParameter("projectId") @RelativePath("..") final String projectId,
710707
@QueryParameter("credentialsId") @RelativePath("..") final String credentialsId) {
711-
checkPermissions();
708+
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
712709
ListBoxModel items = new ListBoxModel();
713710
items.add("");
714711
try {
@@ -727,8 +724,7 @@ public ListBoxModel doFillTemplateItems(
727724
}
728725

729726
public FormValidation doCheckRegion(@QueryParameter String value) {
730-
checkPermissions();
731-
if (value.equals("")) {
727+
if (StringUtils.isEmpty(value)) {
732728
return FormValidation.error("Please select a region...");
733729
}
734730
return FormValidation.ok();
@@ -739,7 +735,7 @@ public ListBoxModel doFillZoneItems(
739735
@QueryParameter("projectId") @RelativePath("..") final String projectId,
740736
@QueryParameter("region") final String region,
741737
@QueryParameter("credentialsId") @RelativePath("..") final String credentialsId) {
742-
checkPermissions();
738+
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
743739
ListBoxModel items = new ListBoxModel();
744740
items.add("");
745741
try {
@@ -761,8 +757,7 @@ public ListBoxModel doFillZoneItems(
761757
}
762758

763759
public FormValidation doCheckZone(@QueryParameter String value) {
764-
checkPermissions();
765-
if (value.equals("")) {
760+
if (StringUtils.isEmpty(value)) {
766761
return FormValidation.error("Please select a zone...");
767762
}
768763
return FormValidation.ok();
@@ -773,7 +768,7 @@ public ListBoxModel doFillMachineTypeItems(
773768
@QueryParameter("projectId") @RelativePath("..") final String projectId,
774769
@QueryParameter("zone") final String zone,
775770
@QueryParameter("credentialsId") @RelativePath("..") final String credentialsId) {
776-
checkPermissions();
771+
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
777772
ListBoxModel items = new ListBoxModel();
778773
items.add("");
779774
try {
@@ -795,8 +790,7 @@ public ListBoxModel doFillMachineTypeItems(
795790
}
796791

797792
public FormValidation doCheckMachineType(@QueryParameter String value) {
798-
checkPermissions();
799-
if (value.equals("")) {
793+
if (StringUtils.isEmpty(value)) {
800794
return FormValidation.error("Please select a machine type...");
801795
}
802796
return FormValidation.ok();
@@ -807,7 +801,7 @@ public ListBoxModel doFillMinCpuPlatformItems(
807801
@QueryParameter("projectId") @RelativePath("..") final String projectId,
808802
@QueryParameter("zone") final String zone,
809803
@QueryParameter("credentialsId") @RelativePath("..") final String credentialsId) {
810-
checkPermissions();
804+
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
811805
ListBoxModel items = new ListBoxModel();
812806
items.add("");
813807
try {
@@ -833,7 +827,7 @@ public ListBoxModel doFillBootDiskTypeItems(
833827
@QueryParameter("projectId") @RelativePath("..") final String projectId,
834828
@QueryParameter("zone") String zone,
835829
@QueryParameter("credentialsId") @RelativePath("..") final String credentialsId) {
836-
checkPermissions();
830+
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
837831
ListBoxModel items = new ListBoxModel();
838832
try {
839833
ComputeClient compute = computeClient(context, credentialsId);
@@ -856,7 +850,7 @@ public ListBoxModel doFillBootDiskTypeItems(
856850
public ListBoxModel doFillBootDiskSourceImageProjectItems(
857851
@AncestorInPath Jenkins context,
858852
@QueryParameter("projectId") @RelativePath("..") final String projectId) {
859-
checkPermissions();
853+
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
860854
ListBoxModel items = new ListBoxModel();
861855
items.add("");
862856
items.add(projectId);
@@ -867,8 +861,7 @@ public ListBoxModel doFillBootDiskSourceImageProjectItems(
867861
}
868862

869863
public FormValidation doCheckBootDiskSourceImageProject(@QueryParameter String value) {
870-
checkPermissions();
871-
if (value.equals("")) {
864+
if (StringUtils.isEmpty(value)) {
872865
return FormValidation.warning("Please select source image project...");
873866
}
874867
return FormValidation.ok();
@@ -878,7 +871,7 @@ public ListBoxModel doFillBootDiskSourceImageNameItems(
878871
@AncestorInPath Jenkins context,
879872
@QueryParameter("bootDiskSourceImageProject") final String projectId,
880873
@QueryParameter("credentialsId") @RelativePath("..") final String credentialsId) {
881-
checkPermissions();
874+
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
882875
ListBoxModel items = new ListBoxModel();
883876
items.add("");
884877
try {
@@ -899,8 +892,7 @@ public ListBoxModel doFillBootDiskSourceImageNameItems(
899892
}
900893

901894
public FormValidation doCheckBootDiskSourceImageName(@QueryParameter String value) {
902-
checkPermissions();
903-
if (value.equals("")) {
895+
if (StringUtils.isEmpty(value)) {
904896
return FormValidation.warning("Please select source image...");
905897
}
906898
return FormValidation.ok();
@@ -912,7 +904,7 @@ public FormValidation doCheckBootDiskSizeGbStr(
912904
@QueryParameter("bootDiskSourceImageProject") final String projectId,
913905
@QueryParameter("bootDiskSourceImageName") final String imageName,
914906
@QueryParameter("credentialsId") @RelativePath("..") final String credentialsId) {
915-
checkPermissions();
907+
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
916908
if (Strings.isNullOrEmpty(credentialsId)
917909
|| Strings.isNullOrEmpty(projectId)
918910
|| Strings.isNullOrEmpty(imageName)) return FormValidation.ok();
@@ -936,7 +928,6 @@ public FormValidation doCheckBootDiskSizeGbStr(
936928

937929
public FormValidation doCheckLabelString(
938930
@QueryParameter String value, @QueryParameter Node.Mode mode) {
939-
checkPermissions();
940931
if (mode == Node.Mode.EXCLUSIVE && (value == null || value.trim().isEmpty())) {
941932
return FormValidation.warning(
942933
"You may want to assign labels to this node;"
@@ -949,7 +940,6 @@ public FormValidation doCheckCreateSnapshot(
949940
@AncestorInPath Jenkins context,
950941
@QueryParameter boolean value,
951942
@QueryParameter("oneShot") boolean oneShot) {
952-
checkPermissions();
953943
if (!oneShot && value) {
954944
return FormValidation.error(Messages.InstanceConfiguration_SnapshotConfigError());
955945
}
@@ -960,7 +950,6 @@ public FormValidation doCheckNumExecutorsStr(
960950
@AncestorInPath Jenkins context,
961951
@QueryParameter String value,
962952
@QueryParameter("oneShot") boolean oneShot) {
963-
checkPermissions();
964953
int numExecutors = intOrDefault(value, DEFAULT_NUM_EXECUTORS);
965954
if (numExecutors < 1) {
966955
return FormValidation.error(

‎src/main/java/com/google/jenkins/plugins/computeengine/SharedVpcNetworkConfiguration.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import hudson.Extension;
2121
import hudson.RelativePath;
2222
import hudson.util.FormValidation;
23+
import jenkins.model.Jenkins;
2324
import lombok.Getter;
2425
import org.kohsuke.stapler.DataBoundConstructor;
2526
import org.kohsuke.stapler.QueryParameter;
@@ -47,15 +48,15 @@ public String getDisplayName() {
4748
}
4849

4950
public FormValidation doCheckProjectId(@QueryParameter String value) {
50-
checkPermissions();
51+
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
5152
if (Strings.isNullOrEmpty(value)) {
5253
return FormValidation.error("Project ID required");
5354
}
5455
return FormValidation.ok();
5556
}
5657

5758
public FormValidation doCheckSubnetworkName(@QueryParameter String value) {
58-
checkPermissions();
59+
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
5960
if (Strings.isNullOrEmpty(value)) {
6061
return FormValidation.error("Subnetwork name required");
6162
}
@@ -69,7 +70,7 @@ public FormValidation doCheckSubnetworkName(@QueryParameter String value) {
6970
public FormValidation doCheckRegion(
7071
@QueryParameter String value,
7172
@QueryParameter("region") @RelativePath("..") final String region) {
72-
checkPermissions();
73+
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
7374
if (Strings.isNullOrEmpty(region)
7475
|| Strings.isNullOrEmpty(value)
7576
|| !region.endsWith(value)) {

‎src/main/java/com/google/jenkins/plugins/computeengine/SshConfiguration.java‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import hudson.Extension;
2727
import hudson.model.Describable;
2828
import hudson.model.Descriptor;
29-
import hudson.model.Item;
3029
import hudson.security.ACL;
3130
import hudson.util.FormValidation;
3231
import hudson.util.ListBoxModel;
@@ -95,10 +94,7 @@ public static final class DescriptorImpl extends Descriptor<SshConfiguration> {
9594

9695
public ListBoxModel doFillCustomPrivateKeyCredentialsIdItems(
9796
@AncestorInPath Jenkins context, @QueryParameter String customPrivateKeyCredentialsId) {
98-
checkPermissions();
99-
if (context == null || !context.hasPermission(Item.CONFIGURE)) {
100-
return new StandardListBoxModel();
101-
}
97+
checkPermissions(context, Jenkins.ADMINISTER);
10298

10399
StandardListBoxModel result = new StandardListBoxModel();
104100

@@ -116,7 +112,7 @@ public FormValidation doCheckCustomPrivateKeyCredentialsId(
116112
@QueryParameter String value,
117113
@QueryParameter("customPrivateKeyCredentialsId") String customPrivateKeyCredentialsId)
118114
throws IOException {
119-
checkPermissions();
115+
checkPermissions(Jenkins.get(), Jenkins.ADMINISTER);
120116

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

0 commit comments

Comments
 (0)