Skip to content

Commit

Permalink
[JENKINS-75073] Missing serverUrl in fillCredentialsIdItems cause som…
Browse files Browse the repository at this point in the history 
…e credentials missing in the initial configuration

If the number of server instances is only one, when a new Jenkins project is created the server list combobox is not available. This causes, in the UI component that loads credentials, to pass the stapler parameter serverUrl as an empty string. In this scenario, serverUrl must be taken from the first of configured endpoints so that it can properly filter the list of credentials.
  • Loading branch information
@nfalco79
nfalco79 committed Jan 9, 2025
1 parent 43d58dd commit 2a4408e
Show file tree
Hide file tree
Showing 16 changed files with 605 additions and 588 deletions.
7 changes: 3 additions & 4 deletions src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketGitSCMBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,9 @@ public BitbucketGitSCMBuilder(@NonNull BitbucketSCMSource scmSource, @NonNull SC
this.scmSource = scmSource;

String serverURL = scmSource.getServerUrl();
AbstractBitbucketEndpoint endpoint = BitbucketEndpointConfiguration.get().findEndpoint(serverURL);
if (endpoint == null) {
endpoint = new BitbucketServerEndpoint(null, serverURL, false, null);
}
AbstractBitbucketEndpoint endpoint = BitbucketEndpointConfiguration.get()
.findEndpoint(serverURL)
.orElse(new BitbucketServerEndpoint(null, serverURL, false, null));

String repositoryURL = endpoint.getRepositoryUrl(scmSource.getRepoOwner(), scmSource.getRepository());
if (BitbucketApiUtils.isCloud(endpoint.getServerUrl())) {
Expand Down
43 changes: 26 additions & 17 deletions src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMNavigator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.interceptor.RequirePOST;

import static com.cloudbees.jenkins.plugins.bitbucket.impl.util.BitbucketApiUtils.getFromBitbucket;

Expand Down Expand Up @@ -290,7 +291,7 @@ public String getServerUrl() {
@DataBoundSetter
public void setServerUrl(String serverUrl) {
serverUrl = BitbucketEndpointConfiguration.normalizeServerUrl(serverUrl);
if (!StringUtils.equals(this.serverUrl, serverUrl)) {
if (serverUrl != null && !StringUtils.equals(this.serverUrl, serverUrl)) {

Check warning on line 294 in src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMNavigator.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 294 is only partially covered, one branch is missing
this.serverUrl = serverUrl;
resetId();
}
Expand Down Expand Up @@ -384,14 +385,14 @@ public String getPattern() {
@DataBoundSetter
public void setBitbucketServerUrl(String url) {
url = BitbucketEndpointConfiguration.normalizeServerUrl(url);
AbstractBitbucketEndpoint endpoint = BitbucketEndpointConfiguration.get().findEndpoint(url);
if (endpoint != null) {
// we have a match
setServerUrl(url);
return;
url = StringUtils.defaultIfBlank(url, BitbucketCloudEndpoint.SERVER_URL);
AbstractBitbucketEndpoint endpoint = BitbucketEndpointConfiguration.get()
.findEndpoint(url)
.orElse(null);
if (endpoint == null) {
LOGGER.log(Level.WARNING, "Call to legacy setBitbucketServerUrl({0}) method is configuring a url missing "
+ "from the global configuration.", url);
}
LOGGER.log(Level.WARNING, "Call to legacy setBitbucketServerUrl({0}) method is configuring a url missing "
+ "from the global configuration.", url);
setServerUrl(url);
}

Expand All @@ -400,7 +401,10 @@ public void setBitbucketServerUrl(String url) {
@RestrictedSince("2.2.0")
@CheckForNull
public String getBitbucketServerUrl() {
if (BitbucketEndpointConfiguration.get().findEndpoint(serverUrl) instanceof BitbucketCloudEndpoint) {
if (BitbucketEndpointConfiguration.get()
.findEndpoint(serverUrl)
.filter(BitbucketCloudEndpoint.class::isInstance)
.isPresent()) {
return null;
}
return serverUrl;
Expand Down Expand Up @@ -629,7 +633,7 @@ public String getIconClassName() {
@Override
public SCMNavigator newInstance(String name) {
BitbucketSCMNavigator instance = new BitbucketSCMNavigator(StringUtils.defaultString(name));
instance.setTraits((List) getTraitsDefaults());
instance.setTraits(getTraitsDefaults());
return instance;
}

Expand All @@ -647,16 +651,21 @@ public ListBoxModel doFillServerUrlItems(@AncestorInPath SCMSourceOwner context)
return BitbucketEndpointConfiguration.get().getEndpointItems();
}

@RequirePOST
@SuppressWarnings("unused") // used By stapler
public FormValidation doCheckCredentialsId(@AncestorInPath SCMSourceOwner context, @QueryParameter String serverUrl, @QueryParameter String value) {
return BitbucketCredentials.checkCredentialsId(context, value, serverUrl);
public static FormValidation doCheckCredentialsId(@AncestorInPath SCMSourceOwner context,
@QueryParameter(fixEmpty = true, value = "serverUrl") String serverURL,
@QueryParameter String value) {
return BitbucketCredentials.checkCredentialsId(context, value, serverURL);
}

@RequirePOST
@SuppressWarnings("unused") // used By stapler
public static FormValidation doCheckMirrorId(@QueryParameter String value, @QueryParameter String serverUrl) {
public static FormValidation doCheckMirrorId(@QueryParameter String value,

Check warning

Code scanning / Jenkins Security Scan

Stapler: Missing permission check Warning

Potential missing permission check in DescriptorImpl#doCheckMirrorId
@QueryParameter(fixEmpty = true, value = "serverUrl") String serverURL) {
if (!value.isEmpty()) {
BitbucketServerWebhookImplementation webhookImplementation =
BitbucketServerEndpoint.findWebhookImplementation(serverUrl);
BitbucketServerEndpoint.findWebhookImplementation(serverURL);

Check warning on line 668 in src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMNavigator.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines 

Lines 636-668 are not covered by tests
if (webhookImplementation == BitbucketServerWebhookImplementation.PLUGIN) {
return FormValidation.error("Mirror can only be used with native webhooks");
}
Expand All @@ -666,13 +675,13 @@ public static FormValidation doCheckMirrorId(@QueryParameter String value, @Quer

@SuppressWarnings("unused") // used By stapler
public ListBoxModel doFillCredentialsIdItems(@AncestorInPath SCMSourceOwner context,
@QueryParameter String serverUrl) {
return BitbucketCredentials.fillCredentialsIdItems(context, serverUrl);
@QueryParameter(fixEmpty = true, value = "serverUrl") String serverURL) {
return BitbucketCredentials.fillCredentialsIdItems(context, serverURL);
}

@SuppressWarnings("unused") // used By stapler
public ListBoxModel doFillMirrorIdItems(@AncestorInPath SCMSourceOwner context,
@QueryParameter String serverUrl,
@QueryParameter(fixEmpty = true, value = "serverUrl") String serverUrl,
@QueryParameter String credentialsId,
@QueryParameter String repoOwner)
throws FormFillFailure {
Expand Down
20 changes: 14 additions & 6 deletions src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,10 @@ public void setTraits(@CheckForNull List<SCMSourceTrait> traits) {
@DataBoundSetter
public void setBitbucketServerUrl(String url) {
url = BitbucketEndpointConfiguration.normalizeServerUrl(url);
AbstractBitbucketEndpoint endpoint = BitbucketEndpointConfiguration.get().findEndpoint(url);
url = StringUtils.defaultIfBlank(url, BitbucketCloudEndpoint.SERVER_URL);
AbstractBitbucketEndpoint endpoint = BitbucketEndpointConfiguration.get()
.findEndpoint(url)
.orElse(null);
if (endpoint != null) {
// we have a match
setServerUrl(endpoint.getServerUrl());
Expand All @@ -418,7 +421,10 @@ public void setBitbucketServerUrl(String url) {
@CheckForNull
public String getBitbucketServerUrl() {
String serverUrl = getServerUrl();
if (BitbucketEndpointConfiguration.get().findEndpoint(serverUrl) instanceof BitbucketCloudEndpoint) {
if (BitbucketEndpointConfiguration.get()
.findEndpoint(serverUrl)
.filter(BitbucketCloudEndpoint.class::isInstance)
.isPresent()) {
return null;
}
return serverUrl;
Expand Down Expand Up @@ -1336,8 +1342,8 @@ public String getDisplayName() {
@SuppressWarnings("unused") // used By stapler
public FormValidation doCheckCredentialsId(@CheckForNull @AncestorInPath SCMSourceOwner context,
@QueryParameter String value,
@QueryParameter String serverUrl) {
return BitbucketCredentials.checkCredentialsId(context, value, serverUrl);
@QueryParameter(fixEmpty = true, value = "serverUrl") String serverURL) {
return BitbucketCredentials.checkCredentialsId(context, value, serverURL);
}

@SuppressWarnings("unused") // used By stapler
Expand All @@ -1353,11 +1359,13 @@ public static FormValidation doCheckServerUrl(@AncestorInPath SCMSourceOwner con
return FormValidation.ok();
}

@RequirePOST
@SuppressWarnings("unused") // used By stapler
public static FormValidation doCheckMirrorId(@QueryParameter String value, @QueryParameter String serverUrl) {
public static FormValidation doCheckMirrorId(@QueryParameter String value,

Check warning

Code scanning / Jenkins Security Scan

Stapler: Missing permission check Warning

Potential missing permission check in DescriptorImpl#doCheckMirrorId
@QueryParameter(fixEmpty = true, value = "serverUrl") String serverURL) {
if (!value.isEmpty()) {
BitbucketServerWebhookImplementation webhookImplementation =
BitbucketServerEndpoint.findWebhookImplementation(serverUrl);
BitbucketServerEndpoint.findWebhookImplementation(serverURL);

Check warning on line 1368 in src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSource.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines 

Lines 1346-1368 are not covered by tests
if (webhookImplementation == BitbucketServerWebhookImplementation.PLUGIN) {
return FormValidation.error("Mirror can only be used with native webhooks");
}
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSourceBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
*/
package com.cloudbees.jenkins.plugins.bitbucket;

import com.cloudbees.jenkins.plugins.bitbucket.endpoints.BitbucketEndpointConfiguration;
import edu.umd.cs.findbugs.annotations.CheckForNull;
import edu.umd.cs.findbugs.annotations.NonNull;
import jenkins.scm.api.trait.SCMSourceBuilder;
Expand Down Expand Up @@ -74,7 +73,7 @@ public BitbucketSCMSourceBuilder(@CheckForNull String id, @NonNull String server
@NonNull String repoName, @CheckForNull String mirrorId) {
super(BitbucketSCMSource.class, repoName);
this.id = id;
this.serverUrl = BitbucketEndpointConfiguration.normalizeServerUrl(serverUrl);
this.serverUrl = serverUrl;
this.credentialsId = credentialsId;
this.mirrorId = mirrorId;
this.repoOwner = repoOwner;
Expand Down
12 changes: 7 additions & 5 deletions src/main/java/com/cloudbees/jenkins/plugins/bitbucket/client/BitbucketCloudApiFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,16 @@ protected boolean isMatch(@Nullable String serverUrl) {
@Override
protected BitbucketApi create(@Nullable String serverUrl, @Nullable BitbucketAuthenticator authenticator,
@NonNull String owner, @CheckForNull String projectKey, @CheckForNull String repository) {
AbstractBitbucketEndpoint endpoint = BitbucketEndpointConfiguration.get().findEndpoint(BitbucketCloudEndpoint.SERVER_URL);
AbstractBitbucketEndpoint endpoint = BitbucketEndpointConfiguration.get()
.findEndpoint(BitbucketCloudEndpoint.SERVER_URL)
.orElse(null);
boolean enableCache = false;
int teamCacheDuration = 0;
int repositoriesCacheDuration = 0;
if (endpoint != null && endpoint instanceof BitbucketCloudEndpoint) {
enableCache = ((BitbucketCloudEndpoint) endpoint).isEnableCache();
teamCacheDuration = ((BitbucketCloudEndpoint) endpoint).getTeamCacheDuration();
repositoriesCacheDuration = ((BitbucketCloudEndpoint) endpoint).getRepositoriesCacheDuration();
if (endpoint instanceof BitbucketCloudEndpoint cloudEndpoint) {
enableCache = cloudEndpoint.isEnableCache();
teamCacheDuration = cloudEndpoint.getTeamCacheDuration();
repositoriesCacheDuration = cloudEndpoint.getRepositoriesCacheDuration();

Check warning on line 34 in src/main/java/com/cloudbees/jenkins/plugins/bitbucket/client/BitbucketCloudApiFactory.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines 

Lines 25-34 are not covered by tests
}
return new BitbucketCloudApiClient(
enableCache, teamCacheDuration, repositoriesCacheDuration,
Expand Down
9 changes: 6 additions & 3 deletions ...ain/java/com/cloudbees/jenkins/plugins/bitbucket/endpoints/AbstractBitbucketEndpoint.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,11 @@ public void setBitbucketJenkinsRootUrl(String bitbucketJenkinsRootUrl) {
*/
@NonNull
public String getEndpointJenkinsRootUrl() {
if (StringUtils.isBlank(bitbucketJenkinsRootUrl))
if (StringUtils.isBlank(bitbucketJenkinsRootUrl)) {
return ClassicDisplayURLProvider.get().getRoot();
else
} else {
return bitbucketJenkinsRootUrl;
}
}

/**
Expand All @@ -172,7 +173,9 @@ public static String getEndpointJenkinsRootUrl(String serverUrl) {
// Note: do not pre-initialize to the global value, so it can be
// reconfigured on the fly.

AbstractBitbucketEndpoint endpoint = BitbucketEndpointConfiguration.get().findEndpoint(serverUrl);
AbstractBitbucketEndpoint endpoint = BitbucketEndpointConfiguration.get()
.findEndpoint(serverUrl)
.orElse(null);
if (endpoint != null) {
return endpoint.getEndpointJenkinsRootUrl();
}
Expand Down
73 changes: 37 additions & 36 deletions ...ava/com/cloudbees/jenkins/plugins/bitbucket/endpoints/BitbucketEndpointConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.ListIterator;
import java.util.Locale;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import jenkins.model.GlobalConfiguration;
Expand Down Expand Up @@ -85,6 +85,7 @@ public static BitbucketEndpointConfiguration get() {
public Permission getRequiredGlobalConfigPagePermission() {
return Jenkins.MANAGE;
}

/**
* Called from a {@code readResolve()} method only to convert the old {@code bitbucketServerUrl} field into the new
* {@code serverUrl} field. When called from {@link ACL#SYSTEM} this will update the configuration with the
Expand All @@ -96,9 +97,10 @@ public Permission getRequiredGlobalConfigPagePermission() {
@Restricted(NoExternalUse.class) // only for plugin internal use.
@NonNull
public String readResolveServerUrl(@CheckForNull String bitbucketServerUrl) {
String serverUrl = normalizeServerUrl(bitbucketServerUrl);
AbstractBitbucketEndpoint endpoint = findEndpoint(serverUrl);
String serverUrl = StringUtils.defaultIfBlank(bitbucketServerUrl, BitbucketCloudEndpoint.SERVER_URL);
AbstractBitbucketEndpoint endpoint = findEndpoint(serverUrl).orElse(null);
if (endpoint == null && ACL.SYSTEM.equals(Jenkins.getAuthentication())) {
serverUrl = normalizeServerUrl(serverUrl);
if (BitbucketCloudEndpoint.SERVER_URL.equals(serverUrl)
|| BitbucketCloudEndpoint.BAD_SERVER_URL.equals(serverUrl)) {
// exception case
Expand All @@ -107,7 +109,7 @@ public String readResolveServerUrl(@CheckForNull String bitbucketServerUrl) {
addEndpoint(new BitbucketServerEndpoint(null, serverUrl, false, null));
}
}
return endpoint == null ? serverUrl : endpoint.getServerUrl();
return endpoint == null ? serverUrl : endpoint.getServerUrl();
}

/**
Expand Down Expand Up @@ -238,66 +240,65 @@ public boolean removeEndpoint(@NonNull AbstractBitbucketEndpoint endpoint) {
/**
* Removes an endpoint.
*
* @param serverUrl the server URL to remove.
* @param serverURL the server URL to remove.
* @return {@code true} if the list of endpoints was modified
*/
public synchronized boolean removeEndpoint(@CheckForNull String serverUrl) {
serverUrl = normalizeServerUrl(serverUrl);
boolean modified = false;
List<AbstractBitbucketEndpoint> endpoints = new ArrayList<>(getEndpoints());
for (Iterator<AbstractBitbucketEndpoint> iterator = endpoints.iterator(); iterator.hasNext(); ) {
if (serverUrl.equals(iterator.next().getServerUrl())) {
iterator.remove();
modified = true;
}
}
setEndpoints(endpoints);
public synchronized boolean removeEndpoint(@CheckForNull String serverURL) {
String fixedServerURL = normalizeServerUrl(serverURL);
List<AbstractBitbucketEndpoint> newEndpoints = new ArrayList<>(getEndpoints());
boolean modified = newEndpoints.removeIf(endpoint -> Objects.equals(fixedServerURL, endpoint.getServerUrl()));
setEndpoints(newEndpoints);
return modified;
}

/**
* Checks to see if the supplied server URL is defined in the global configuration.
*
* @param serverUrl the server url to check.
* @param serverURL the server url to check.
* @return the global configuration for the specified server url or {@code null} if not defined.
*/
@CheckForNull
public synchronized AbstractBitbucketEndpoint findEndpoint(@CheckForNull String serverUrl) {
serverUrl = normalizeServerUrl(serverUrl);
public synchronized Optional<AbstractBitbucketEndpoint> findEndpoint(@CheckForNull String serverURL) {
serverURL = normalizeServerUrl(serverURL);
for (AbstractBitbucketEndpoint endpoint : getEndpoints()) {
if (serverUrl.equals(endpoint.getServerUrl())) {
return endpoint;
if (Objects.equals(serverURL, endpoint.getServerUrl())) {
return Optional.of(endpoint);
}
}
return null;
return Optional.empty();
}

/**
* Checks to see if the supplied server URL is defined in the global configuration.
*
* @param serverUrl the server url to check.
* @param serverURL the server url to check.
* @param clazz the class to check.
* @return the global configuration for the specified server url or {@code null} if not defined.
*/
public synchronized Optional<AbstractBitbucketEndpoint> findEndpoint(@CheckForNull String serverUrl,
Class<? extends AbstractBitbucketEndpoint> clazz) {
return getEndpoints().stream()
public synchronized <T extends AbstractBitbucketEndpoint> Optional<T> findEndpoint(@CheckForNull String serverURL,
Class<T> clazz) {
return findEndpoint(serverURL)
.filter(clazz::isInstance)
.filter(endpoint -> normalizeServerUrl(serverUrl).equals(endpoint.getServerUrl()))
.findFirst();
.map(clazz::cast);
}

@NonNull
public synchronized AbstractBitbucketEndpoint getDefaultEndpoint() {
return getEndpoints().get(0);
}

/**
* Fix a serverUrl.
*
* @param serverUrl the server URL.
* @param serverURL the server URL.
* @return the normalized server URL.
*/
@NonNull
public static String normalizeServerUrl(@CheckForNull String serverUrl) {
serverUrl = StringUtils.defaultIfBlank(serverUrl, BitbucketCloudEndpoint.SERVER_URL);
@CheckForNull
public static String normalizeServerUrl(@CheckForNull String serverURL) {
if (StringUtils.isBlank(serverURL)) {
return null;
}
try {
URI uri = new URI(serverUrl).normalize();
URI uri = new URI(serverURL).normalize();
String scheme = uri.getScheme();
if ("http".equals(scheme) || "https".equals(scheme)) {
// we only expect http / https, but also these are the only ones where we know the authority
Expand All @@ -311,7 +312,7 @@ public static String normalizeServerUrl(@CheckForNull String serverUrl) {
} else if ("https".equals(scheme) && port == 443) {
port = -1;
}
serverUrl = new URI(
serverURL = new URI(
scheme,
uri.getUserInfo(),
host,
Expand All @@ -324,7 +325,7 @@ public static String normalizeServerUrl(@CheckForNull String serverUrl) {
} catch (URISyntaxException e) {
// ignore, this was a best effort tidy-up
}
return serverUrl.replaceAll("/$", "");
return serverURL.replaceAll("/$", "");
}

}
Loading

0 comments on commit 2a4408e

Please sign in to comment.