Skip to content

Commit

Permalink
Use current context for validation functions (#1608)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Dohbedoh
Dohbedoh authored Oct 10, 2024
1 parent 32d31ac commit 11898cf
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 21 deletions.
52 changes: 40 additions & 12 deletions src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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");

Expand All @@ -964,6 +967,7 @@ public FormValidation doTestConnection(
namespace,
Util.fixEmpty(serverCertificate),
Util.fixEmpty(credentialsId),
owner,
skipTlsVerify,
connectionTimeout,
readTimeout,
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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)));
Expand Down Expand Up @@ -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 {
Expand Down
34 changes: 31 additions & 3 deletions src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesFactoryAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
18 changes: 12 additions & 6 deletions src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloudFIPSTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 11898cf

Please sign in to comment.