diff --git a/pom.xml b/pom.xml index 2523f7527..f43f97162 100644 --- a/pom.xml +++ b/pom.xml @@ -52,6 +52,10 @@ github 1.31.0 + + org.jenkins-ci.plugins.workflow + workflow-support + io.jsonwebtoken jjwt-api @@ -92,6 +96,24 @@ mockito-core test + + org.jenkins-ci.plugins.workflow + workflow-basic-steps + test + + + org.jenkins-ci.plugins + git + tests + test + + + org.apache.httpcomponents + httpclient + + + + diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubAppCredentials.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubAppCredentials.java index 1dccb8e11..843178454 100644 --- a/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubAppCredentials.java +++ b/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubAppCredentials.java @@ -1,13 +1,13 @@ package org.jenkinsci.plugins.github_branch_source; import com.cloudbees.plugins.credentials.CredentialsScope; -import com.cloudbees.plugins.credentials.CredentialsSnapshotTaker; import com.cloudbees.plugins.credentials.common.StandardUsernamePasswordCredentials; import com.cloudbees.plugins.credentials.impl.BaseStandardCredentials; import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.Extension; +import hudson.Functions; import hudson.Util; import hudson.remoting.Channel; import hudson.util.FormValidation; @@ -15,14 +15,23 @@ import hudson.util.Secret; import java.io.IOException; import java.io.Serializable; +import java.time.Duration; +import java.time.Instant; import java.util.List; +import java.util.concurrent.TimeUnit; +import java.util.logging.Level; +import java.util.logging.Logger; + +import jenkins.security.SlaveToMasterCallable; +import jenkins.util.JenkinsJVM; +import net.sf.json.JSONObject; +import org.jenkinsci.plugins.workflow.support.concurrent.Timeout; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; import org.kohsuke.github.GHApp; import org.kohsuke.github.GHAppInstallation; import org.kohsuke.github.GHAppInstallationToken; import org.kohsuke.github.GitHub; -import org.kohsuke.github.GitHubBuilder; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.DataBoundSetter; import org.kohsuke.stapler.QueryParameter; @@ -34,6 +43,8 @@ @SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "XStream") public class GitHubAppCredentials extends BaseStandardCredentials implements StandardUsernamePasswordCredentials { + private static final Logger LOGGER = Logger.getLogger(GitHubAppCredentials.class.getName()); + private static final String ERROR_AUTHENTICATING_GITHUB_APP = "Couldn't authenticate with GitHub app ID %s"; private static final String NOT_INSTALLED = ", has it been installed to your GitHub organisation / user?"; @@ -49,8 +60,7 @@ public class GitHubAppCredentials extends BaseStandardCredentials implements Sta private String owner; - private transient String cachedToken; - private transient long tokenCacheTime; + private transient AppInstallationToken cachedToken; @DataBoundConstructor @SuppressWarnings("unused") // by stapler @@ -104,15 +114,21 @@ public void setOwner(String owner) { } @SuppressWarnings("deprecation") // preview features are required for GitHub app integration, GitHub api adds deprecated to all preview methods - static String generateAppInstallationToken(String appId, String appPrivateKey, String apiUrl, String owner) { - try { + static AppInstallationToken generateAppInstallationToken(String appId, String appPrivateKey, String apiUrl, String owner) { + // We expect this to be fast but if anything hangs in here we do not want to block indefinitely + try (Timeout timeout = Timeout.limit(30, TimeUnit.SECONDS)) { String jwtToken = createJWT(appId, appPrivateKey); GitHub gitHubApp = Connector .createGitHubBuilder(apiUrl) .withJwtToken(jwtToken) .build(); - GHApp app = gitHubApp.getApp(); + GHApp app; + try { + app = gitHubApp.getApp(); + } catch (IOException e) { + throw new IllegalArgumentException(String.format(ERROR_AUTHENTICATING_GITHUB_APP, appId), e); + } List appInstallations = app.listInstallations().asList(); if (appInstallations.isEmpty()) { @@ -123,20 +139,44 @@ static String generateAppInstallationToken(String appId, String appPrivateKey, S appInstallation = appInstallations.get(0); } else { appInstallation = appInstallations.stream() - .filter(installation -> installation.getAccount().getLogin().equals(owner)) - .findAny() - .orElseThrow(() -> new IllegalArgumentException(String.format(ERROR_NOT_INSTALLED, appId))); + .filter(installation -> installation.getAccount().getLogin().equals(owner)) + .findAny() + .orElseThrow(() -> new IllegalArgumentException(String.format(ERROR_NOT_INSTALLED, appId))); } GHAppInstallationToken appInstallationToken = appInstallation - .createToken(appInstallation.getPermissions()) - .create(); + .createToken(appInstallation.getPermissions()) + .create(); + + long expiration = getExpirationSeconds(appInstallationToken); + AppInstallationToken token = new AppInstallationToken(appInstallationToken.getToken(), + expiration); + LOGGER.log(Level.FINER, + "Generated App Installation Token for app ID {0}", + appId); + + return token; + } catch (Exception e) { + throw new IllegalArgumentException("Failed to generate GitHub App installation token for app ID " + appId , e); + } + } - return appInstallationToken.getToken(); - } catch (IOException e) { - throw new IllegalArgumentException(String.format(ERROR_AUTHENTICATING_GITHUB_APP, appId), e); + private static long getExpirationSeconds(GHAppInstallationToken appInstallationToken) { + try { + return appInstallationToken.getExpiresAt() + .toInstant() + .getEpochSecond(); + } catch (Exception e) { + // if we fail to calculate the expiration, guess at a reasonable value. + LOGGER.log(Level.WARNING, + "Unable to get GitHub App installation token expiration", + e); + return Instant.now().getEpochSecond() + AppInstallationToken.NOT_STALE_MINIMUM_SECONDS; } + } + @NonNull String actualApiUri() { + return Util.fixEmpty(apiUri) == null ? "https://api.github.com" : apiUri; } /** @@ -145,20 +185,34 @@ static String generateAppInstallationToken(String appId, String appPrivateKey, S @NonNull @Override public Secret getPassword() { - if (Util.fixEmpty(apiUri) == null) { - apiUri = "https://api.github.com"; - } - - long now = System.currentTimeMillis(); String appInstallationToken; - if (cachedToken != null && now - tokenCacheTime < JwtHelper.VALIDITY_MS /* extra buffer */ / 2) { - appInstallationToken = cachedToken; - } else { - appInstallationToken = generateAppInstallationToken(appID, privateKey.getPlainText(), apiUri, owner); - cachedToken = appInstallationToken; - tokenCacheTime = now; + synchronized (this) { + try { + if (cachedToken == null || cachedToken.isStale()) { + LOGGER.log(Level.FINE, "Generating App Installation Token for app ID {0}", appID); + cachedToken = generateAppInstallationToken(appID, + privateKey.getPlainText(), + actualApiUri(), + owner); + LOGGER.log(Level.FINER, "Retrieved GitHub App Installation Token for app ID {0}", appID); + } + } catch (Exception e) { + if (cachedToken != null && !cachedToken.isExpired()) { + // Requesting a new token failed. If the cached token is not expired, continue to use it. + // This minimizes failures due to occasional network instability, + // while only slightly increasing the chance that tokens will expire while in use. + LOGGER.log(Level.WARNING, + "Failed to generate new GitHub App Installation Token for app ID " + appID + ": cached token is stale but has not expired", + e); + } else { + throw e; + } + } + appInstallationToken = cachedToken.getToken(); } + LOGGER.log(Level.FINEST, "Returned GitHub App Installation Token for app ID {0}", appID); + return Secret.fromString(appInstallationToken); } @@ -171,57 +225,244 @@ public String getUsername() { return appID; } + private AppInstallationToken getCachedToken() { + synchronized (this) { + return cachedToken; + } + } + + static class AppInstallationToken implements Serializable { + /** + * {@link #getPassword()} checks that the token is still valid before returning it. + * The token generally will not expire for at least this amount of time after it is returned. + * + * Using a larger value will result in longer time-to-live for the token, but also more network + * calls related to getting new tokens. Setting a smaller value will result in less token generation + * but runs the the risk of the token expiring while it is still being used. + * + * The time-to-live for the token may be less than this if the initial expiration for the token when + * it is returned from GitHub is less than this or if the token is kept and due to failures + * while retrieving a new token. + */ + static final long STALE_BEFORE_EXPIRATION_SECONDS = Duration.ofMinutes(45).getSeconds(); + + /** + * Any token older than this is considered stale. + * + * This is a back stop to ensure that, in case of unforeseen error, + * expired tokens are not accidentally retained past their expiration. + */ + static final long STALE_AFTER_SECONDS = Duration.ofMinutes(30).getSeconds(); + + /** + * When a token is retrieved it cannot got stale for at least this many seconds. + * + * Prevents continuous refreshing of credentials. + * Non-final for testing purposes. + * This value takes precedence over {@link #STALE_BEFORE_EXPIRATION_SECONDS}. + * {@link #STALE_BEFORE_EXPIRATION_SECONDS} takes precedence over this value. + * Minimum value of 1. + */ + static long NOT_STALE_MINIMUM_SECONDS = Duration.ofMinutes(1).getSeconds(); + + private final String token; + private final long expirationEpochSeconds; + private final long staleEpochSeconds; + + /** + * Create a AppInstallationToken instance. + * + * Tokens will always become stale after {@link #STALE_AFTER_SECONDS} seconds. + * Tokens will not become stale for at least {@link #NOT_STALE_MINIMUM_SECONDS}, + * as long as that does not exceed {@link #STALE_AFTER_SECONDS}. + * Within the bounds of {@link #NOT_STALE_MINIMUM_SECONDS} and {@link #STALE_AFTER_SECONDS}, + * tokens will become stale {@link #STALE_BEFORE_EXPIRATION_SECONDS} seconds before they expire. + * + * @param token the token string + * @param expirationEpochSeconds the time in epoch seconds that this token will expire + */ + public AppInstallationToken(String token, long expirationEpochSeconds) { + long now = Instant.now().getEpochSecond(); + long minimumAllowedAge = Math.max(1, NOT_STALE_MINIMUM_SECONDS); + long maximumAllowedAge = Math.max(1, 1 + STALE_AFTER_SECONDS); + + // Tokens go stale a while before they will expire + long secondsUntilStale = (expirationEpochSeconds - now) - STALE_BEFORE_EXPIRATION_SECONDS; + + // Tokens are never stale as soon as they are made + if (secondsUntilStale < minimumAllowedAge) { + secondsUntilStale = minimumAllowedAge; + } + + // Tokens have a maximum age at which they go stale + if (secondsUntilStale > maximumAllowedAge) { + secondsUntilStale = maximumAllowedAge; + } + + LOGGER.log(Level.FINER, "Token will become stale after " + secondsUntilStale + " seconds"); + + this.token = token; + this.expirationEpochSeconds = expirationEpochSeconds; + this.staleEpochSeconds = now + secondsUntilStale; + } + + public String getToken() { + return token; + } + + /** + * Whether a token is stale and should be replaced with a new token. + * + * {@link #getPassword()} checks that the token is not "stale" before returning it. + * If a token is "stale" if it has expired, exceeded {@link #STALE_AFTER_SECONDS}, or + * will expire in less than {@link #STALE_BEFORE_EXPIRATION_SECONDS}. + * + * @return {@code true} if token should be refreshed, otherwise {@code false}. + */ + public boolean isStale() { + return Instant.now().getEpochSecond() >= staleEpochSeconds; + } + + public boolean isExpired() { + return Instant.now().getEpochSecond() >= expirationEpochSeconds; + } + + long getTokenStaleEpochSeconds() { + return staleEpochSeconds; + } + } + /** - * Ensures that the credentials state as serialized via Remoting to an agent includes fields which are {@code transient} for purposes of XStream. - * This provides a ~2× performance improvement over reconstructing the object without that state, - * in the normal case that {@link #cachedToken} is valid and will remain valid for the brief time that elapses before the agent calls {@link #getPassword}: + * Ensures that the credentials state as serialized via Remoting to an agent calls back to the controller. + * Benefits: * - * @see CredentialsSnapshotTaker */ private Object writeReplace() { if (/* XStream */Channel.current() == null) { return this; } - return new Replacer(this); - } - - private static final class Replacer implements Serializable { - - private final CredentialsScope scope; - private final String id; - private final String description; - private final String appID; - private final Secret privateKey; - private final String apiUri; - private final String owner; - private final String cachedToken; - private final long tokenCacheTime; - - Replacer(GitHubAppCredentials onMaster) { - scope = onMaster.getScope(); - id = onMaster.getId(); - description = onMaster.getDescription(); - appID = onMaster.appID; - privateKey = onMaster.privateKey; - apiUri = onMaster.apiUri; - owner = onMaster.owner; - cachedToken = onMaster.cachedToken; - tokenCacheTime = onMaster.tokenCacheTime; - } - - private Object readResolve() { - GitHubAppCredentials clone = new GitHubAppCredentials(scope, id, description, appID, privateKey); - clone.apiUri = apiUri; - clone.owner = owner; - clone.cachedToken = cachedToken; - clone.tokenCacheTime = tokenCacheTime; - return clone; - } - - } + return new DelegatingGitHubAppCredentials(this); + } + + private static final class DelegatingGitHubAppCredentials extends BaseStandardCredentials implements StandardUsernamePasswordCredentials { + + private final String appID; + /** + * An encrypted form of all data needed to refresh the token. + * Used to prevent {@link GetToken} from being abused by compromised build agents. + */ + private final String tokenRefreshData; + private AppInstallationToken cachedToken; + + private transient Channel ch; + + DelegatingGitHubAppCredentials(GitHubAppCredentials onMaster) { + super(onMaster.getScope(), onMaster.getId(), onMaster.getDescription()); + JenkinsJVM.checkJenkinsJVM(); + appID = onMaster.appID; + JSONObject j = new JSONObject(); + j.put("appID", appID); + j.put("privateKey", onMaster.privateKey.getPlainText()); + j.put("apiUri", onMaster.actualApiUri()); + j.put("owner", onMaster.owner); + tokenRefreshData = Secret.fromString(j.toString()).getEncryptedValue(); + + // Check token is valid before sending it to the agent. + // Ensuring the cached token is not stale before sending it to agents keeps agents from having to + // immediately refresh the token. + // This is intentionally only a best-effort attempt. + // If this fails, the agent will fallback to making the request (which may or may not fail). + try { + LOGGER.log(Level.FINEST, "Checking App Installation Token for app ID {0} before sending to agent", onMaster.appID); + onMaster.getPassword(); + } catch (Exception e) { + LOGGER.log(Level.WARNING, "Failed to update stale GitHub App installation token for app ID " + onMaster.getAppID() + " before sending to agent", e); + } + + cachedToken = onMaster.getCachedToken(); + } + + private Object readResolve() { + JenkinsJVM.checkNotJenkinsJVM(); + synchronized (this) { + ch = Channel.currentOrFail(); + } + return this; + } + + @NonNull + @Override + public String getUsername() { + return appID; + } + + @Override + public Secret getPassword() { + JenkinsJVM.checkNotJenkinsJVM(); + try { + String appInstallationToken; + synchronized (this) { + try { + if (cachedToken == null || cachedToken.isStale()) { + LOGGER.log(Level.FINE, "Generating App Installation Token for app ID {0} on agent", appID); + cachedToken = ch.call(new GetToken(tokenRefreshData)); + LOGGER.log(Level.FINER, "Retrieved GitHub App Installation Token for app ID {0} on agent", appID); + } + } catch (Exception e) { + if (cachedToken != null && !cachedToken.isExpired()) { + // Requesting a new token failed. If the cached token is not expired, continue to use it. + // This minimizes failures due to occasional network instability, + // while only slightly increasing the chance that tokens will expire while in use. + LOGGER.log(Level.WARNING, + "Failed to generate new GitHub App Installation Token for app ID " + appID + " on agent: cached token is stale but has not expired"); + // Logging the exception here caused a security exeception when trying to read the agent logs during testing + // Added the exception to a secondary log message that can be viewed if it is needed + LOGGER.log(Level.FINER, () -> Functions.printThrowable(e)); + } else { + throw e; + } + } + appInstallationToken = cachedToken.getToken(); + } + + LOGGER.log(Level.FINEST, "Returned GitHub App Installation Token for app ID {0} on agent", appID); + + return Secret.fromString(appInstallationToken); + } catch (IOException | InterruptedException x) { + throw new RuntimeException(x); + } + } + + private static final class GetToken extends SlaveToMasterCallable { + + private final String data; + + GetToken(String data) { + this.data = data; + } + + @Override + public AppInstallationToken call() throws RuntimeException { + JenkinsJVM.checkJenkinsJVM(); + JSONObject fields = JSONObject.fromObject(Secret.fromString(data).getPlainText()); + LOGGER.log(Level.FINE, "Generating App Installation Token for app ID {0} for agent", fields.get("appID")); + AppInstallationToken token = generateAppInstallationToken( + (String)fields.get("appID"), + (String)fields.get("privateKey"), + (String)fields.get("apiUri"), + (String)fields.get("owner")); + LOGGER.log(Level.FINER, + "Retrieved GitHub App Installation Token for app ID {0} for agent", + fields.get("appID")); + return token; + } + } + } /** * {@inheritDoc} diff --git a/src/test/java/org/jenkinsci/plugins/github_branch_source/GithubAppCredentialsAppInstallationTokenTest.java b/src/test/java/org/jenkinsci/plugins/github_branch_source/GithubAppCredentialsAppInstallationTokenTest.java new file mode 100644 index 000000000..2c36704e0 --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/github_branch_source/GithubAppCredentialsAppInstallationTokenTest.java @@ -0,0 +1,69 @@ +package org.jenkinsci.plugins.github_branch_source; + +import org.junit.Test; + +import java.time.Duration; +import java.time.Instant; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; + +public class GithubAppCredentialsAppInstallationTokenTest { + + @Test + public void testAppInstallationTokenStale() throws Exception { + + GitHubAppCredentials.AppInstallationToken token; + long now; + + now = Instant.now().getEpochSecond(); + token = new GitHubAppCredentials.AppInstallationToken("", now); + assertThat(token.isStale(), is(false)); + assertThat(token.getTokenStaleEpochSeconds(), equalTo(now + GitHubAppCredentials.AppInstallationToken.NOT_STALE_MINIMUM_SECONDS)); + + now = Instant.now().getEpochSecond(); + token = new GitHubAppCredentials.AppInstallationToken("", + now + Duration.ofMinutes(15).getSeconds()); + assertThat(token.isStale(), is(false)); + assertThat(token.getTokenStaleEpochSeconds(), equalTo(now + GitHubAppCredentials.AppInstallationToken.NOT_STALE_MINIMUM_SECONDS)); + + now = Instant.now().getEpochSecond(); + token = new GitHubAppCredentials.AppInstallationToken("", + now + GitHubAppCredentials.AppInstallationToken.STALE_BEFORE_EXPIRATION_SECONDS + 2); + assertThat(token.isStale(), is(false)); + assertThat(token.getTokenStaleEpochSeconds(), equalTo(now + GitHubAppCredentials.AppInstallationToken.NOT_STALE_MINIMUM_SECONDS)); + + now = Instant.now().getEpochSecond(); + token = new GitHubAppCredentials.AppInstallationToken("", + now + GitHubAppCredentials.AppInstallationToken.STALE_BEFORE_EXPIRATION_SECONDS + Duration + .ofMinutes(7) + .getSeconds()); + assertThat(token.isStale(), is(false)); + assertThat(token.getTokenStaleEpochSeconds(), + equalTo(now + Duration.ofMinutes(7).getSeconds())); + + now = Instant.now().getEpochSecond(); + token = new GitHubAppCredentials.AppInstallationToken("", + now + Duration.ofMinutes(90).getSeconds()); + assertThat(token.isStale(), is(false)); + assertThat(token.getTokenStaleEpochSeconds(), + equalTo(now + GitHubAppCredentials.AppInstallationToken.STALE_AFTER_SECONDS + 1)); + + long notStaleSeconds = GitHubAppCredentials.AppInstallationToken.NOT_STALE_MINIMUM_SECONDS; + try { + // Should revert to 1 second minimum + GitHubAppCredentials.AppInstallationToken.NOT_STALE_MINIMUM_SECONDS = -10; + + now = Instant.now().getEpochSecond(); + token = new GitHubAppCredentials.AppInstallationToken("", now); + assertThat(token.isStale(), is(false)); + assertThat(token.getTokenStaleEpochSeconds(), equalTo(now + 1)); + + // Verify goes stale + Thread.sleep(1000); + assertThat(token.isStale(), is(true)); + } finally { + GitHubAppCredentials.AppInstallationToken.NOT_STALE_MINIMUM_SECONDS = notStaleSeconds; + } + } +} \ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/github_branch_source/GithubAppCredentialsTest.java b/src/test/java/org/jenkinsci/plugins/github_branch_source/GithubAppCredentialsTest.java new file mode 100644 index 000000000..0730593ee --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/github_branch_source/GithubAppCredentialsTest.java @@ -0,0 +1,327 @@ +package org.jenkinsci.plugins.github_branch_source; + +import com.cloudbees.plugins.credentials.CredentialsProvider; +import com.cloudbees.plugins.credentials.CredentialsScope; +import com.cloudbees.plugins.credentials.CredentialsStore; +import com.cloudbees.plugins.credentials.domains.Domain; +import hudson.logging.LogRecorder; +import hudson.logging.LogRecorderManager; +import hudson.model.ParametersDefinitionProperty; +import hudson.model.Result; +import hudson.model.Slave; +import hudson.model.StringParameterDefinition; +import hudson.util.Secret; +import jenkins.plugins.git.GitSampleRepoRule; +import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; +import org.jenkinsci.plugins.workflow.job.WorkflowJob; +import org.jenkinsci.plugins.workflow.job.WorkflowRun; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; + +import java.time.Duration; +import java.time.Instant; +import java.time.format.DateTimeFormatter; +import java.time.temporal.ChronoUnit; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Date; +import java.util.List; +import java.util.logging.Formatter; +import java.util.logging.Level; +import java.util.logging.LogRecord; +import java.util.logging.SimpleFormatter; +import java.util.stream.Collectors; + +import static com.github.tomakehurst.wiremock.client.WireMock.*; +import static org.hamcrest.Matchers.*; + +public class GithubAppCredentialsTest extends AbstractGitHubWireMockTest { + + private static Slave agent; + private static final String myAppCredentialsId = "myAppCredentialsId"; + private static CredentialsStore store; + private static GitHubAppCredentials appCredentials; + private static LogRecorder logRecorder; + + @Rule + public GitSampleRepoRule sampleRepo = new GitSampleRepoRule(); + + @BeforeClass + public static void setUpJenkins() throws Exception { + //Add credential (Must have valid private key for Jwt to work, but App doesn't have to actually exist) + store = CredentialsProvider.lookupStores(r.jenkins).iterator().next(); + appCredentials = new GitHubAppCredentials( + CredentialsScope.GLOBAL, myAppCredentialsId, "sample", "54321", Secret.fromString(JwtHelperTest.PKCS8_PRIVATE_KEY)); + appCredentials.setOwner("cloudbeers"); + store.addCredentials(Domain.global(), appCredentials); + + // Add agent + agent = r.createOnlineSlave(); + agent.setLabelString("my-agent"); + + // Would use LoggerRule, but need to get agent logs as well + LogRecorderManager mgr = r.jenkins.getLog(); + logRecorder = new LogRecorder(GitHubAppCredentials.class.getName()); + mgr.logRecorders.put(GitHubAppCredentials.class.getName(), logRecorder); + LogRecorder.Target t = new LogRecorder.Target(GitHubAppCredentials.class.getName(), Level.FINE); + logRecorder.targets.add(t); + logRecorder.save(); + t.enable(); + } + + @Before + public void setUpWireMock() throws Exception { + //Add wiremock responses for App, App Installation, and App Installation Token + githubApi.stubFor( + get(urlEqualTo("/app")) + .willReturn( + aResponse() + .withHeader("Content-Type", "application/json; charset=utf-8") + .withBodyFile("../AppCredentials/files/body-mapping-githubapp-app.json"))); + githubApi.stubFor( + get(urlEqualTo("/app/installations")) + .willReturn( + aResponse() + .withHeader("Content-Type", "application/json; charset=utf-8") + .withBodyFile("../AppCredentials/files/body-mapping-githubapp-installations.json"))); + + final String scenarioName = "credentials-accesstoken"; + + githubApi.stubFor( + post(urlEqualTo("/app/installations/654321/access_tokens")) + .inScenario(scenarioName) + .whenScenarioStateIs("Started") + .willSetStateTo("1") + .withRequestBody( + equalToJson( + "{\"permissions\":{\"pull_requests\":\"write\",\"metadata\":\"read\",\"checks\":\"write\",\"contents\":\"read\"}}", true, false)) + .willReturn( + aResponse() + .withHeader("Content-Type", "application/json; charset=utf-8") + .withBody("{\n" + + " \"token\": \"super-secret-token\",\n" + + // This token will go stale at the soonest allowed time but will not expire for the duration of the test + " \"expires_at\": \"" + printDate(new Date(System.currentTimeMillis() + Duration.ofMinutes(10).toMillis())) + "\"" + // 2019-08-10T05:54:58Z + "}"))); + + // Force an error to test fallback refreshing from agent + githubApi.stubFor( + post(urlEqualTo("/app/installations/654321/access_tokens")) + .inScenario(scenarioName) + .whenScenarioStateIs("1") + .willSetStateTo("2") + .withRequestBody( + equalToJson( + "{\"permissions\":{\"pull_requests\":\"write\",\"metadata\":\"read\",\"checks\":\"write\",\"contents\":\"read\"}}", true, false)) + .willReturn( + aResponse() + .withStatus(404) + .withStatusMessage("404 Not Found") + .withHeader("Content-Type", "application/json; charset=utf-8") + .withBody("{\"message\": \"File not found\"}"))); + + // Force an error to test fallback to returning unexpired token on agent + githubApi.stubFor( + post(urlEqualTo("/app/installations/654321/access_tokens")) + .inScenario(scenarioName) + .whenScenarioStateIs("2") + .willSetStateTo("3") + .withRequestBody( + equalToJson( + "{\"permissions\":{\"pull_requests\":\"write\",\"metadata\":\"read\",\"checks\":\"write\",\"contents\":\"read\"}}", true, false)) + .willReturn( + aResponse() + .withStatus(404) + .withStatusMessage("404 Not Found") + .withHeader("Content-Type", "application/json; charset=utf-8") + .withBody("{\"message\": \"File not found\"}"))); + + // return an expired token on controller + githubApi.stubFor( + post(urlEqualTo("/app/installations/654321/access_tokens")) + .inScenario(scenarioName) + .whenScenarioStateIs("3") + .willSetStateTo("4") + .withRequestBody( + equalToJson( + "{\"permissions\":{\"pull_requests\":\"write\",\"metadata\":\"read\",\"checks\":\"write\",\"contents\":\"read\"}}", true, false)) + .willReturn( + aResponse() + .withHeader("Content-Type", "application/json; charset=utf-8") + .withBody("{\n" + + " \"token\": \"super-secret-token\",\n" + + // token is already expired, but will not go stale for at least the minimum time + // This is a valid scenario - clocks are not always properly synchronized. + " \"expires_at\": \"" + printDate(new Date()) + "\"" + // 2019-08-10T05:54:58Z + "}"))); + + // Force an error to test non-fallback scenario and refreshing on agent + githubApi.stubFor( + post(urlEqualTo("/app/installations/654321/access_tokens")) + .inScenario(scenarioName) + .whenScenarioStateIs("4") + .willSetStateTo("5") + .withRequestBody( + equalToJson( + "{\"permissions\":{\"pull_requests\":\"write\",\"metadata\":\"read\",\"checks\":\"write\",\"contents\":\"read\"}}", true, false)) + .willReturn( + aResponse() + .withStatus(404) + .withStatusMessage("404 Not Found") + .withHeader("Content-Type", "application/json; charset=utf-8") + .withBody("{\"message\": \"File not found\"}"))); + + // Valid token retirieved on agent + githubApi.stubFor( + post(urlEqualTo("/app/installations/654321/access_tokens")) + .inScenario(scenarioName) + .whenScenarioStateIs("5") + .willSetStateTo("6") + .withRequestBody( + equalToJson( + "{\"permissions\":{\"pull_requests\":\"write\",\"metadata\":\"read\",\"checks\":\"write\",\"contents\":\"read\"}}", true, false)) + .willReturn( + aResponse() + .withHeader("Content-Type", "application/json; charset=utf-8") + .withBody("{\n" + + " \"token\": \"super-secret-token\",\n" + + " \"expires_at\": \"" + printDate(new Date()) + "\"" + // 2019-08-10T05:54:58Z + "}"))); + + // Valid token retirieved on controller + githubApi.stubFor( + post(urlEqualTo("/app/installations/654321/access_tokens")) + .inScenario(scenarioName) + .whenScenarioStateIs("6") + .willSetStateTo("7") // setting this to non-existant state means any extra requests will fail + .withRequestBody( + equalToJson( + "{\"permissions\":{\"pull_requests\":\"write\",\"metadata\":\"read\",\"checks\":\"write\",\"contents\":\"read\"}}", true, false)) + .willReturn( + aResponse() + .withHeader("Content-Type", "application/json; charset=utf-8") + .withBody("{\n" + + " \"token\": \"super-secret-token\",\n" + + " \"expires_at\": \"" + printDate(new Date()) + "\"" + // 2019-08-10T05:54:58Z + "}"))); + } + + @Test + public void testAgentRefresh() throws Exception { + long notStaleSeconds = GitHubAppCredentials.AppInstallationToken.NOT_STALE_MINIMUM_SECONDS; + try { + appCredentials.setApiUri(githubApi.baseUrl()); + + // We want to demonstrate successful caching without waiting for a the default 1 minute + // Must set this to a large enough number to avoid flaky test + GitHubAppCredentials.AppInstallationToken.NOT_STALE_MINIMUM_SECONDS = 5; + + final String gitCheckoutStep = String.format( + " git url: REPO, credentialsId: '%s'", + myAppCredentialsId); + + final String jenkinsfile = String.join( + "\n", + "// run checkout several times", + "node ('my-agent') {", + " echo 'First Checkout on agent should use cached token passed via remoting'", + gitCheckoutStep, + " echo 'Multiple checkouts in quick succession should use cached token'", + gitCheckoutStep, + " sleep " + (GitHubAppCredentials.AppInstallationToken.NOT_STALE_MINIMUM_SECONDS + 2), + " echo 'Checkout after token is stale refreshes via remoting - fallback due to unexpired token'", + gitCheckoutStep, + " echo 'Checkout after error will refresh again on controller - new token expired but not stale'", + gitCheckoutStep, + " sleep " + (GitHubAppCredentials.AppInstallationToken.NOT_STALE_MINIMUM_SECONDS + 2), + " echo 'Checkout after token is stale refreshes via remoting - error on controller is not catastrophic'", + gitCheckoutStep, + " echo 'Checkout after error will refresh again on controller - new token expired but not stale'", + gitCheckoutStep, + " echo 'Multiple checkouts in quick succession should use cached token'", + gitCheckoutStep, + "}"); + + + // Create a repo with the above Jenkinsfile + sampleRepo.init(); + sampleRepo.write("Jenkinsfile", jenkinsfile); + sampleRepo.git("add", "Jenkinsfile"); + sampleRepo.git("commit", "--message=init"); + + // Create a pipeline job that points the above repo + WorkflowJob job = r.createProject(WorkflowJob.class, "test-creds"); + job.setDefinition(new CpsFlowDefinition(jenkinsfile, false)); + job.addProperty(new ParametersDefinitionProperty( + new StringParameterDefinition("REPO", sampleRepo.toString()))); + WorkflowRun run = job.scheduleBuild2(0).waitForStart(); + r.waitUntilNoActivity(); + + assertThat(run.getResult(), equalTo(Result.SUCCESS)); + System.out.println(JenkinsRule.getLog(run)); + + List credentialsLog = getOutputLines(); + + //Verify correct messages from GitHubAppCredential logger indicating token was retrieved on agent + assertThat("Creds should cache on master, pass to agent, and refresh agent from master once", + credentialsLog, contains( + // (agent log added out of order, see below) + "Generating App Installation Token for app ID 54321 on agent", // 1 + "Failed to generate new GitHub App Installation Token for app ID 54321 on agent: cached token is stale but has not expired", // 2 + "Generating App Installation Token for app ID 54321 on agent", // 3 + // node ('my-agent') { + // checkout scm + "Generating App Installation Token for app ID 54321", + // checkout scm + // (No token generation) + // sleep + // checkout scm + "Generating App Installation Token for app ID 54321", + // (error forced by wiremock) + "Failed to generate new GitHub App Installation Token for app ID 54321: cached token is stale but has not expired", + // (error forced by wiremock - failed refresh on the agent) + // "Generating App Installation Token for app ID 54321 on agent", // 1 + "Generating App Installation Token for app ID 54321 for agent", + // (agent log added out of order) "Keeping cached GitHub App Installation Token for app ID 54321 on agent: token is stale but has not expired", // 2 + // checkout scm - refresh on controller + "Generating App Installation Token for app ID 54321", + // sleep + // checkout scm + "Generating App Installation Token for app ID 54321", + // (error forced by wiremock) + "Failed to update stale GitHub App installation token for app ID 54321 before sending to agent", + // "Generating App Installation Token for app ID 54321 on agent", // 3 + "Generating App Installation Token for app ID 54321 for agent", + // checkout scm - refresh on controller + "Generating App Installation Token for app ID 54321" + // checkout scm + // (No token generation) + )); + } finally { + GitHubAppCredentials.AppInstallationToken.NOT_STALE_MINIMUM_SECONDS = notStaleSeconds; + logRecorder.doClear(); + } + } + + private List getOutputLines() { + final Formatter formatter = new SimpleFormatter(); + List result = new ArrayList<>(logRecorder.getLogRecords()); + List agentLogs = logRecorder.getSlaveLogRecords().get(agent.toComputer()); + if (agentLogs != null) { + result.addAll(agentLogs); + } + Collections.reverse(result); + return result.stream() + .map(formatter::formatMessage) + .collect(Collectors.toList()); + } + + static String printDate(Date dt) { + return DateTimeFormatter.ISO_INSTANT.format(Instant.ofEpochMilli(dt.getTime()).truncatedTo( + ChronoUnit.SECONDS)); + } + +} \ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/github_branch_source/JwtHelperTest.java b/src/test/java/org/jenkinsci/plugins/github_branch_source/JwtHelperTest.java index 8735e2a2d..d63381dda 100644 --- a/src/test/java/org/jenkinsci/plugins/github_branch_source/JwtHelperTest.java +++ b/src/test/java/org/jenkinsci/plugins/github_branch_source/JwtHelperTest.java @@ -23,7 +23,7 @@ public class JwtHelperTest { public ExpectedException expectedException = ExpectedException.none(); // https://stackoverflow.com/a/22176759/4951015 - private static final String PKCS8_PRIVATE_KEY = "-----BEGIN PRIVATE KEY-----\n" + + public static final String PKCS8_PRIVATE_KEY = "-----BEGIN PRIVATE KEY-----\n" + // Windows line ending "MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQD7vHsVwyDV8cj7\r\n" + // This should also work @@ -54,7 +54,7 @@ public class JwtHelperTest { "Nw9bewRvqjySBlDJ9/aNSeEY\n" + "-----END PRIVATE KEY-----"; - private static final String PKCS8_PUBLIC_KEY = "-----BEGIN PUBLIC KEY-----\n" + + public static final String PKCS8_PUBLIC_KEY = "-----BEGIN PUBLIC KEY-----\n" + "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA+7x7FcMg1fHI++ckeFlp\n" + "eq5YH/3uc5ngYLZtDwoJ4rXEmw+RVN999NQz2OWuwLalQTsBJxNeC/V0u6L+tVPs\n" + "dZuGkJqkf3T0GgNmsodnGD7M1jUDKeNgKwyH4Ow4Uq77ESSh7Gz15T46nnNLnS6o\n" + diff --git a/src/test/resources/api/AppCredentials/files/body-mapping-githubapp-app.json b/src/test/resources/api/AppCredentials/files/body-mapping-githubapp-app.json new file mode 100644 index 000000000..eb2aaee6e --- /dev/null +++ b/src/test/resources/api/AppCredentials/files/body-mapping-githubapp-app.json @@ -0,0 +1,39 @@ +{ + "id": 54321, + "node_id": "MDM6QXBwMzI2MTY=", + "owner": { + "login": "cloudbeers", + "id": 4181899, + "node_id": "asdfasdfasdf", + "url": "https://api.github.com/orgs/cloudbeers", + "html_url": "https://github.com/cloudbeers", + "followers_url": "https://api.github.com/users/cloudbeers/followers", + "following_url": "https://api.github.com/users/cloudbeers/following{/other_user}", + "gists_url": "https://api.github.com/users/cloudbeers/gists{/gist_id}", + "starred_url": "https://api.github.com/users/cloudbeers/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/cloudbeers/subscriptions", + "organizations_url": "https://api.github.com/users/cloudbeers/orgs", + "repos_url": "https://api.github.com/users/cloudbeers/repos", + "events_url": "https://api.github.com/users/cloudbeers/events{/privacy}", + "received_events_url": "https://api.github.com/users/cloudbeers/received_events", + "type": "Organization", + "site_admin": false + }, + "name": "Bogus-Development", + "description": "", + "external_url": "https://bogus.domain.com", + "html_url": "https://github.com/apps/bogus-development", + "created_at": "2019-06-10T04:21:41Z", + "updated_at": "2019-06-10T04:21:41Z", + "permissions": { + "checks": "write", + "contents": "read", + "metadata": "read", + "pull_requests": "write" + }, + "events": [ + "pull_request", + "push" + ], + "installations_count": 1 +} \ No newline at end of file diff --git a/src/test/resources/api/AppCredentials/files/body-mapping-githubapp-installations.json b/src/test/resources/api/AppCredentials/files/body-mapping-githubapp-installations.json new file mode 100644 index 000000000..7a463e643 --- /dev/null +++ b/src/test/resources/api/AppCredentials/files/body-mapping-githubapp-installations.json @@ -0,0 +1,84 @@ +[ + { + "id": 654321, + "account": { + "login": "cloudbeers", + "id": 4181899, + "node_id": "asdfasdfasdf", + "url": "https://api.github.com/orgs/cloudbeers", + "html_url": "https://github.com/cloudbeers", + "followers_url": "https://api.github.com/users/cloudbeers/followers", + "following_url": "https://api.github.com/users/cloudbeers/following{/other_user}", + "gists_url": "https://api.github.com/users/cloudbeers/gists{/gist_id}", + "starred_url": "https://api.github.com/users/cloudbeers/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/cloudbeers/subscriptions", + "organizations_url": "https://api.github.com/users/cloudbeers/orgs", + "repos_url": "https://api.github.com/users/cloudbeers/repos", + "events_url": "https://api.github.com/users/cloudbeers/events{/privacy}", + "received_events_url": "https://api.github.com/users/cloudbeers/received_events", + "type": "Organization", + "site_admin": false + }, + "repository_selection": "selected", + "access_tokens_url": "https://api.github.com/app/installations/654321/access_tokens", + "repositories_url": "https://api.github.com/installation/repositories", + "html_url": "https://github.com/organizations/cloudbeers/settings/installations/654321", + "app_id": 54321, + "target_id": 4181899, + "target_type": "Organization", + "permissions": { + "checks": "write", + "pull_requests": "write", + "contents": "read", + "metadata": "read" + }, + "events": [ + "pull_request", + "push" + ], + "created_at": "2019-07-04T01:19:36.000Z", + "updated_at": "2019-07-30T22:48:09.000Z", + "single_file_name": null + }, + { + "id": 754321, + "account": { + "login": "bogus", + "id": 0, + "node_id": "asdfasdfasdff", + "url": "https://api.github.com/orgs/bogus", + "html_url": "https://github.com/bogus", + "followers_url": "https://api.github.com/users/bogus/followers", + "following_url": "https://api.github.com/users/bogus/following{/other_user}", + "gists_url": "https://api.github.com/users/bogus/gists{/gist_id}", + "starred_url": "https://api.github.com/users/bogus/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/bogus/subscriptions", + "organizations_url": "https://api.github.com/users/bogus/orgs", + "repos_url": "https://api.github.com/users/v/repos", + "events_url": "https://api.github.com/users/bogus/events{/privacy}", + "received_events_url": "https://api.github.com/users/bogus/received_events", + "type": "Organization", + "site_admin": false + }, + "repository_selection": "selected", + "access_tokens_url": "https://api.github.com/app/installations/754321/access_tokens", + "repositories_url": "https://api.github.com/installation/repositories", + "html_url": "https://github.com/organizations/bogus/settings/installations/754321", + "app_id": 54321, + "target_id": 4181899, + "target_type": "Organization", + "permissions": { + "checks": "write", + "pull_requests": "write", + "contents": "read", + "metadata": "read" + }, + "events": [ + "pull_request", + "push" + ], + "created_at": "2019-07-04T01:19:36.000Z", + "updated_at": "2019-07-30T22:48:09.000Z", + "single_file_name": null + } +] \ No newline at end of file