From 11898cf8fa66fe80969c0d6e0843e66471898be2 Mon Sep 17 00:00:00 2001 From: Allan Burdajewicz Date: Fri, 11 Oct 2024 01:45:02 +1000 Subject: [PATCH] Use current context for validation functions (#1608) --- .../plugins/kubernetes/KubernetesCloud.java | 52 ++++++++++++++----- .../kubernetes/KubernetesFactoryAdapter.java | 34 ++++++++++-- .../kubernetes/KubernetesCloudFIPSTest.java | 18 ++++--- 3 files changed, 83 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java index 45827e0e7c..d93e9c60a9 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java @@ -945,6 +945,7 @@ public static void addAliases() { @RequirePOST @SuppressWarnings("unused") // used by jelly public FormValidation doTestConnection( + @AncestorInPath ItemGroup owner, @QueryParameter String name, @QueryParameter String serverUrl, @QueryParameter String credentialsId, @@ -953,9 +954,11 @@ public FormValidation doTestConnection( @QueryParameter String namespace, @QueryParameter int connectionTimeout, @QueryParameter int readTimeout, - @QueryParameter boolean useJenkinsProxy) - throws Exception { - Jenkins.get().checkPermission(Jenkins.MANAGE); + @QueryParameter boolean useJenkinsProxy) { + + AccessControlled _context = (owner instanceof AccessControlled ? (AccessControlled) owner : Jenkins.get()); + + checkPermission(_context); if (StringUtils.isBlank(name)) return FormValidation.error("name is required"); @@ -964,6 +967,7 @@ public FormValidation doTestConnection( namespace, Util.fixEmpty(serverCertificate), Util.fixEmpty(credentialsId), + owner, skipTlsVerify, connectionTimeout, readTimeout, @@ -994,8 +998,11 @@ public FormValidation doTestConnection( @RequirePOST @SuppressWarnings({"unused", "lgtm[jenkins/csrf]" }) // used by jelly and already fixed jenkins security scan warning - public FormValidation doCheckSkipTlsVerify(@QueryParameter boolean skipTlsVerify) { - Jenkins.get().checkPermission(Jenkins.MANAGE); + public FormValidation doCheckSkipTlsVerify( + @AncestorInPath AccessControlled owner, @QueryParameter boolean skipTlsVerify) { + if (!hasPermission(owner)) { + return FormValidation.ok(); + } try { ensureSkipTlsVerifyInFipsMode(skipTlsVerify); } catch (IllegalArgumentException ex) { @@ -1007,8 +1014,11 @@ public FormValidation doCheckSkipTlsVerify(@QueryParameter boolean skipTlsVerify @RequirePOST @SuppressWarnings({"unused", "lgtm[jenkins/csrf]" }) // used by jelly and already fixed jenkins security scan warning - public FormValidation doCheckServerCertificate(@QueryParameter String serverCertificate) { - Jenkins.get().checkPermission(Jenkins.MANAGE); + public FormValidation doCheckServerCertificate( + @AncestorInPath AccessControlled owner, @QueryParameter String serverCertificate) { + if (!hasPermission(owner)) { + return FormValidation.ok(); + } try { ensureServerCertificateInFipsMode(serverCertificate); } catch (IllegalArgumentException ex) { @@ -1019,8 +1029,11 @@ public FormValidation doCheckServerCertificate(@QueryParameter String serverCert @RequirePOST @SuppressWarnings("unused") // used by jelly - public FormValidation doCheckServerUrl(@QueryParameter String serverUrl) { - Jenkins.get().checkPermission(Jenkins.MANAGE); + public FormValidation doCheckServerUrl( + @AncestorInPath AccessControlled owner, @QueryParameter String serverUrl) { + if (!hasPermission(owner)) { + return FormValidation.ok(); + } try { ensureKubernetesUrlInFipsMode(serverUrl); } catch (IllegalArgumentException ex) { @@ -1032,13 +1045,13 @@ public FormValidation doCheckServerUrl(@QueryParameter String serverUrl) { @RequirePOST @SuppressWarnings("unused") // used by jelly public ListBoxModel doFillCredentialsIdItems( - @AncestorInPath ItemGroup context, @QueryParameter String serverUrl) { - Jenkins.get().checkPermission(Jenkins.MANAGE); + @AncestorInPath ItemGroup owner, @QueryParameter String serverUrl) { + checkPermission((owner instanceof AccessControlled ? (AccessControlled) owner : Jenkins.get())); StandardListBoxModel result = new StandardListBoxModel(); result.includeEmptyValue(); result.includeMatchingAs( ACL.SYSTEM, - context, + owner, StandardCredentials.class, serverUrl != null ? URIRequirementBuilder.fromUri(serverUrl).build() : Collections.EMPTY_LIST, CredentialsMatchers.anyOf(AuthenticationTokens.matcher(KubernetesAuth.class))); @@ -1134,6 +1147,21 @@ private static boolean hasPermission(AccessControlled owner) { } } + private static void checkPermission(AccessControlled owner) { + if (owner instanceof Jenkins) { + // Regular cloud + owner.checkPermission(Jenkins.ADMINISTER); + } else if (owner instanceof Item) { + // Shared cloud (CloudBees CI) + owner.checkPermission(Item.CONFIGURE); + } else { + throw new IllegalArgumentException( + "Unsupported owner type " + (owner == null ? "null" : owner.getClass()) + " (url: " + + Stapler.getCurrentRequest().getOriginalRequestURI() + + "). Please report this issue to the plugin maintainers."); + } + } + @SuppressWarnings("unused") // used by jelly public FormValidation doCheckJenkinsUrl(@QueryParameter String value, @QueryParameter boolean directConnection) throws IOException, ServletException { diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesFactoryAdapter.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesFactoryAdapter.java index 9a3fd85652..e4acdc46b5 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesFactoryAdapter.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesFactoryAdapter.java @@ -10,6 +10,7 @@ import edu.umd.cs.findbugs.annotations.NonNull; import hudson.ProxyConfiguration; import hudson.Util; +import hudson.model.ItemGroup; import hudson.security.ACL; import hudson.util.Secret; import io.fabric8.kubernetes.client.Config; @@ -117,10 +118,35 @@ public KubernetesFactoryAdapter( int maxRequestsPerHost, boolean useJenkinsProxy) throws KubernetesAuthException { + this( + serviceAddress, + namespace, + caCertData, + credentialsId, + Jenkins.get(), + skipTlsVerify, + connectTimeout, + readTimeout, + maxRequestsPerHost, + useJenkinsProxy); + } + + public KubernetesFactoryAdapter( + String serviceAddress, + String namespace, + @CheckForNull String caCertData, + @CheckForNull String credentialsId, + @NonNull ItemGroup context, + boolean skipTlsVerify, + int connectTimeout, + int readTimeout, + int maxRequestsPerHost, + boolean useJenkinsProxy) + throws KubernetesAuthException { this.serviceAddress = serviceAddress; this.namespace = namespace; this.caCertData = decodeBase64IfNeeded(caCertData); - this.auth = AuthenticationTokens.convert(KubernetesAuth.class, resolveCredentials(credentialsId)); + this.auth = AuthenticationTokens.convert(KubernetesAuth.class, resolveCredentials(credentialsId, context)); this.skipTlsVerify = skipTlsVerify; this.connectTimeout = connectTimeout; this.readTimeout = readTimeout; @@ -248,16 +274,18 @@ public String toString() { } @CheckForNull - private static StandardCredentials resolveCredentials(@CheckForNull String credentialsId) + private static StandardCredentials resolveCredentials(@CheckForNull String credentialsId, @NonNull ItemGroup owner) throws KubernetesAuthException { if (credentialsId == null) { return null; } + StandardCredentials c = CredentialsMatchers.firstOrNull( CredentialsProvider.lookupCredentialsInItemGroup( - StandardCredentials.class, Jenkins.get(), ACL.SYSTEM2, Collections.emptyList()), + StandardCredentials.class, owner, ACL.SYSTEM2, Collections.emptyList()), CredentialsMatchers.allOf( AuthenticationTokens.matcher(KubernetesAuth.class), CredentialsMatchers.withId(credentialsId))); + if (c == null) { throw new KubernetesAuthException("No credentials found with id " + credentialsId); } diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloudFIPSTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloudFIPSTest.java index 65492b120f..e9c3797356 100644 --- a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloudFIPSTest.java +++ b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloudFIPSTest.java @@ -81,26 +81,32 @@ public void formValidationTest() throws IOException { .orElseGet(KubernetesCloud.DescriptorImpl::new); assertThat( "Valid url doesn't raise error", - descriptor.doCheckServerUrl("https://eample.org").getMessage(), + descriptor.doCheckServerUrl(r.jenkins, "https://eample.org").getMessage(), nullValue()); assertThat( "Invalid url raises error", - descriptor.doCheckServerUrl("http://eample.org").getMessage(), + descriptor.doCheckServerUrl(r.jenkins, "http://eample.org").getMessage(), notNullValue()); assertThat( "Valid cert doesn't raise error", - descriptor.doCheckServerCertificate(getCert("rsa2048")).getMessage(), + descriptor + .doCheckServerCertificate(r.jenkins, getCert("rsa2048")) + .getMessage(), nullValue()); assertThat( "Invalid cert raises error", - descriptor.doCheckServerCertificate(getCert("rsa1024")).getMessage(), + descriptor + .doCheckServerCertificate(r.jenkins, getCert("rsa1024")) + .getMessage(), notNullValue()); assertThat( "No TLS skip doesn't raise error", - descriptor.doCheckSkipTlsVerify(false).getMessage(), + descriptor.doCheckSkipTlsVerify(r.jenkins, false).getMessage(), nullValue()); assertThat( - "TLS skip raises error", descriptor.doCheckSkipTlsVerify(true).getMessage(), notNullValue()); + "TLS skip raises error", + descriptor.doCheckSkipTlsVerify(r.jenkins, true).getMessage(), + notNullValue()); } private String getCert(String alg) throws IOException {