Skip to content

Commit

Permalink
[JENKINS-61200] in case of authentication issue downloading avatar, r…
Browse files Browse the repository at this point in the history
…efresh credentials and make cache time configurable (#637)

* [JENKINS-61200] in case of authentication issue downloading avatar, refresh credentials and make cache time configurable

Signed-off-by: Olivier Lamy <olamy@apache.org>

* add some logging when authz happen while fetching bb scm source

Signed-off-by: Olivier Lamy <olamy@apache.org>

Signed-off-by: Olivier Lamy <olamy@apache.org>
Co-authored-by: Günter Grodotzki <gunter@grodotzki.com>
  • Loading branch information
olamy and lifeofguenter committed Sep 2, 2022
1 parent 863acff commit beeaa47
Show file tree
Hide file tree
Showing 9 changed files with 149 additions and 72 deletions.
1 change: 1 addition & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@
</configuration>
<executions>
<execution>
<phase>validate</phase>
<goals>
<goal>check</goal>
</goals>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -817,62 +817,75 @@ private void retrieveTags(final BitbucketSCMSourceRequest request) throws IOExce
@Override
protected SCMRevision retrieve(SCMHead head, TaskListener listener) throws IOException, InterruptedException {
final BitbucketApi bitbucket = buildBitbucketClient();
List<? extends BitbucketBranch> branches = bitbucket.getBranches();
if (head instanceof PullRequestSCMHead) {
PullRequestSCMHead h = (PullRequestSCMHead) head;
BitbucketCommit targetRevision = findCommit(h.getTarget().getName(), branches, listener);
if (targetRevision == null) {
LOGGER.log(Level.WARNING, "No branch found in {0}/{1} with name [{2}]",
try {
List<? extends BitbucketBranch> branches = bitbucket.getBranches();
if (head instanceof PullRequestSCMHead) {
PullRequestSCMHead h = (PullRequestSCMHead) head;
BitbucketCommit targetRevision = findCommit(h.getTarget().getName(), branches, listener);
if (targetRevision == null) {
LOGGER.log(Level.WARNING, "No branch found in {0}/{1} with name [{2}]",
new Object[]{repoOwner, repository, h.getTarget().getName()});
return null;
}
BitbucketCommit sourceRevision;
if (bitbucket instanceof BitbucketCloudApiClient) {
branches = head.getOrigin() == SCMHeadOrigin.DEFAULT
return null;
}
BitbucketCommit sourceRevision;
if (bitbucket instanceof BitbucketCloudApiClient) {
branches = head.getOrigin() == SCMHeadOrigin.DEFAULT
? branches
: buildBitbucketClient(h).getBranches();
sourceRevision = findCommit(h.getBranchName(), branches, listener);
} else {
try {
BitbucketPullRequest pr = bitbucket.getPullRequestById(Integer.parseInt(h.getId()));
sourceRevision = findPRCommit(pr, listener);
} catch (NumberFormatException nfe) {
LOGGER.log(Level.WARNING, "Cannot parse the PR id {0}", h.getId());
sourceRevision = null;
sourceRevision = findCommit(h.getBranchName(), branches, listener);
} else {
try {
BitbucketPullRequest pr = bitbucket.getPullRequestById(Integer.parseInt(h.getId()));
sourceRevision = findPRCommit(pr, listener);
} catch (NumberFormatException nfe) {
LOGGER.log(Level.WARNING, "Cannot parse the PR id {0}", h.getId());
sourceRevision = null;
}
}
}
if (sourceRevision == null) {
LOGGER.log(Level.WARNING, "No revision found in {0}/{1} for PR-{2} [{3}]",
if (sourceRevision == null) {
LOGGER.log(Level.WARNING, "No revision found in {0}/{1} for PR-{2} [{3}]",
new Object[]{
h.getRepoOwner(),
h.getRepository(),
h.getId(),
h.getBranchName()
h.getRepoOwner(),
h.getRepository(),
h.getId(),
h.getBranchName()
});
return null;
}
return new PullRequestSCMRevision<>(
return null;
}
return new PullRequestSCMRevision<>(
h,
new BitbucketGitSCMRevision(h.getTarget(), targetRevision),
new BitbucketGitSCMRevision(h, sourceRevision)
);
} else if(head instanceof BitbucketTagSCMHead) {
BitbucketTagSCMHead tagHead = (BitbucketTagSCMHead) head;
List<? extends BitbucketBranch> tags = bitbucket.getTags();
BitbucketCommit revision = findCommit(head.getName(), tags, listener);
if (revision == null) {
LOGGER.log(Level.WARNING, "No tag found in {0}/{1} with name [{2}]", new Object[] { repoOwner, repository, head.getName() });
return null;
}
return new BitbucketTagSCMRevision(tagHead, revision);
} else {
BitbucketCommit revision = findCommit(head.getName(), branches, listener);
if (revision == null) {
LOGGER.log(Level.WARNING, "No branch found in {0}/{1} with name [{2}]",
);
} else if (head instanceof BitbucketTagSCMHead) {
BitbucketTagSCMHead tagHead = (BitbucketTagSCMHead) head;
List<? extends BitbucketBranch> tags = bitbucket.getTags();
BitbucketCommit revision = findCommit(head.getName(), tags, listener);
if (revision == null) {
LOGGER.log(Level.WARNING, "No tag found in {0}/{1} with name [{2}]", new Object[]{repoOwner, repository, head.getName()});
return null;
}
return new BitbucketTagSCMRevision(tagHead, revision);
} else {
BitbucketCommit revision = findCommit(head.getName(), branches, listener);
if (revision == null) {
LOGGER.log(Level.WARNING, "No branch found in {0}/{1} with name [{2}]",
new Object[]{repoOwner, repository, head.getName()});
return null;
return null;
}
return new BitbucketGitSCMRevision(head, revision);
}
} catch (IOException e) {
// here we only want to display the job name to have it in the log
if (e instanceof BitbucketRequestException) {
BitbucketRequestException bre = (BitbucketRequestException) e;
SCMSourceOwner scmSourceOwner = getOwner();
if (bre.getHttpCode() == 401 && scmSourceOwner != null) {
LOGGER.log(Level.WARNING, "BitbucketRequestException: Authz error. Status: 401 for Item '{0}' using credentialId '{1}'",
new Object[]{scmSourceOwner.getFullDisplayName(), getCredentialsId()});
}
}
return new BitbucketGitSCMRevision(head, revision);
throw e;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,16 @@
import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketApi;
import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketApiFactory;
import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketAuthenticator;
import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketRequestException;
import com.cloudbees.jenkins.plugins.bitbucket.avatars.AvatarCache;
import com.cloudbees.jenkins.plugins.bitbucket.avatars.AvatarCacheSource;
import com.cloudbees.plugins.credentials.CredentialsMatchers;
import com.cloudbees.plugins.credentials.CredentialsProvider;
import com.cloudbees.plugins.credentials.common.StandardCredentials;
import com.cloudbees.plugins.credentials.domains.URIRequirementBuilder;
import hudson.model.Item;
import hudson.security.ACL;
import hudson.security.ACLContext;
import java.io.IOException;
import java.io.Serializable;
import java.util.Objects;
Expand Down Expand Up @@ -61,7 +68,7 @@ public BitbucketTeamMetadataAction(String serverUrl, StandardCredentials credent
public static class BitbucketAvatarCacheSource implements AvatarCacheSource, Serializable {
private static final long serialVersionUID = 1L;
private final String serverUrl;
private final StandardCredentials credentials;
private StandardCredentials credentials;
private final String repoOwner;

public BitbucketAvatarCacheSource(String serverUrl, StandardCredentials credentials, String repoOwner) {
Expand All @@ -72,20 +79,66 @@ public BitbucketAvatarCacheSource(String serverUrl, StandardCredentials credenti
}

@Override
public AvatarImage fetch() {
BitbucketAuthenticator authenticator = AuthenticationTokens
.convert(BitbucketAuthenticator.authenticationContext(serverUrl), credentials);
BitbucketApi bitbucket = BitbucketApiFactory.newInstance(serverUrl, authenticator, repoOwner, null);
public AvatarImage fetch(StandardCredentials credentials) {
try {
return bitbucket.getTeamAvatar();
return doFetch(credentials);
} catch (IOException e) {
if (e.getCause() instanceof BitbucketRequestException) {
BitbucketRequestException bre = (BitbucketRequestException) e.getCause();
if (bre.getHttpCode()==401) {
// credentials not updated here maybe we need to refresh them
StandardCredentials standardCredentials = findCredentials();
// try again with refreshed credentials
// TODO compare previous and new token if we really need to try again
if (standardCredentials != null) {
this.credentials = standardCredentials;
try {
return doFetch(this.credentials);
} catch (IOException | InterruptedException ex) {
LOGGER.log(Level.INFO, ex.getClass().getName()+": " + e.getMessage(), e);
}
}
}
}
LOGGER.log(Level.INFO, "IOException: " + e.getMessage(), e);
} catch (InterruptedException e) {
LOGGER.log(Level.INFO, "InterruptedException: " + e.getMessage(), e);
}
return null;
}

private StandardCredentials findCredentials() {
try (ACLContext ctx = ACL.as(ACL.SYSTEM)) {
return CredentialsMatchers.firstOrNull(
CredentialsProvider.lookupCredentials(
credentials.getClass(),
(Item) null, // context
ACL.SYSTEM,
URIRequirementBuilder.fromUri(serverUrl).build()
),
CredentialsMatchers.allOf(
CredentialsMatchers.withId(credentials.getId()),
CredentialsMatchers.anyOf(CredentialsMatchers.instanceOf(credentials.getClass()))
)
);
}
}

private AvatarImage doFetch(StandardCredentials credentials) throws IOException, InterruptedException {
BitbucketAuthenticator authenticator = AuthenticationTokens
.convert(BitbucketAuthenticator.authenticationContext(serverUrl), credentials);
BitbucketApi bitbucket = BitbucketApiFactory.newInstance(serverUrl, authenticator, repoOwner, null);
return bitbucket.getTeamAvatar();
}

@Override
public AvatarImage fetch() {
if(this.credentials==null) {
throw new UnsupportedOperationException("this method can be used only with credentials");
}
return this.fetch(this.credentials);
}

@Override
public String hashKey() {
return "" + serverUrl + "::" + repoOwner + "::" + (credentials != null ? credentials.getId() : "");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketAuthenticator;
import com.cloudbees.plugins.credentials.common.StandardUsernamePasswordCredentials;
import hudson.util.Secret;
import org.apache.http.HttpRequest;
import org.scribe.model.OAuthConfig;
import org.scribe.model.OAuthConstants;
Expand All @@ -20,7 +19,7 @@ public class BitbucketOAuthAuthenticator extends BitbucketAuthenticator {
public BitbucketOAuthAuthenticator(StandardUsernamePasswordCredentials credentials) {
super(credentials);

OAuthConfig config = new OAuthConfig(credentials.getUsername(), Secret.toString(credentials.getPassword()));
OAuthConfig config = new OAuthConfig(credentials.getUsername(), credentials.getPassword().getPlainText());

BitbucketOAuthService OAuthService = (BitbucketOAuthService) new BitbucketOAuth().createService(config);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import com.cloudbees.plugins.credentials.Credentials;
import com.cloudbees.plugins.credentials.CredentialsMatcher;
import com.cloudbees.plugins.credentials.common.UsernamePasswordCredentials;
import hudson.util.Secret;
import java.util.logging.Level;
import java.util.logging.Logger;

Expand All @@ -30,7 +29,7 @@ public boolean matches(Credentials item) {
UsernamePasswordCredentials usernamePasswordCredential = ((UsernamePasswordCredentials) item);
String username = usernamePasswordCredential.getUsername();
boolean isEMail = username.contains(".") && username.contains("@");
boolean validSecretLength = Secret.toString(usernamePasswordCredential.getPassword()).length() == secretLength;
boolean validSecretLength = usernamePasswordCredential.getPassword().getPlainText().length() == secretLength;
boolean validKeyLength = username.length() == keyLength;

return !isEMail && validKeyLength && validSecretLength;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@

import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketAuthenticator;
import com.cloudbees.plugins.credentials.common.StandardUsernamePasswordCredentials;
import hudson.util.Secret;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.apache.http.HttpHost;
Expand Down Expand Up @@ -56,7 +55,7 @@ public class BitbucketUsernamePasswordAuthenticator extends BitbucketAuthenticat
public BitbucketUsernamePasswordAuthenticator(StandardUsernamePasswordCredentials credentials) {
super(credentials);
httpCredentials = new UsernamePasswordCredentials(credentials.getUsername(),
Secret.toString(credentials.getPassword()));
credentials.getPassword().getPlainText());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,13 @@ public class AvatarCache implements UnprotectedRootAction {
/**
* The cache of entries. Unused entries will be removed over time.
*/
private final ConcurrentMap<String, CacheEntry> cache = new ConcurrentHashMap<String, CacheEntry>();
private final ConcurrentMap<String, CacheEntry> cache = new ConcurrentHashMap<>();

/**
* A background thread pool to refresh images.
*/
private final ThreadPoolExecutor service = new ThreadPoolExecutor(CONCURRENT_REQUEST_LIMIT,
CONCURRENT_REQUEST_LIMIT, 1L, TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>(),
CONCURRENT_REQUEST_LIMIT, 1L, TimeUnit.SECONDS, new LinkedBlockingQueue<>(),
new NamingThreadFactory(new DaemonThreadFactory(), getClass().getName()));

/**
Expand Down Expand Up @@ -552,15 +552,15 @@ private synchronized void setFuture(Future<CacheEntry> future) {
}

private synchronized boolean isStale() {
return System.currentTimeMillis() - lastModified > TimeUnit.HOURS.toMillis(1);
return System.currentTimeMillis() - lastModified > TimeUnit.MINUTES.toMillis(Long.getLong(AvatarCache.class.getName()+".stale.ttl",60));
}

private void touch() {
lastAccessed = System.currentTimeMillis();
}

private boolean isUnused() {
return lastAccessed > 0L && System.currentTimeMillis() - lastAccessed > TimeUnit.HOURS.toMillis(2);
return lastAccessed > 0L && System.currentTimeMillis() - lastAccessed > TimeUnit.MINUTES.toMillis(Long.getLong(AvatarCache.class.getName()+".unused.ttl",60));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
*/
package com.cloudbees.jenkins.plugins.bitbucket.avatars;

import com.cloudbees.plugins.credentials.common.StandardCredentials;
import java.awt.image.BufferedImage;

/**
Expand All @@ -49,23 +50,33 @@ public AvatarImage(final BufferedImage image, final long lastModified) {

/**
*
* @deprecated use {@link #fetch(StandardCredentials)}
* Fetch image from source
*
* @return AvatarImage object
*/
public AvatarImage fetch();
@Deprecated
AvatarImage fetch();

/**
*
* Fetch image from source
* @param credentials the credentials to use
* @return AvatarImage object
*/
AvatarImage fetch(StandardCredentials credentials);

/**
* Get unique hashKey for this item
*
* @return AvatarImage object
*/
public String hashKey();
String hashKey();

/**
* Make sure we can fetch
*
* @return true if can fetch
*/
public boolean canFetch();
boolean canFetch();
}
Loading

0 comments on commit beeaa47

Please sign in to comment.