From 4c3a0d329b780767254cb290f619a0cb8d4aa9d4 Mon Sep 17 00:00:00 2001 From: Alex Taylor Date: Mon, 27 Jan 2020 15:24:28 -0500 Subject: [PATCH] added Authentication Check Added additional authentication checks on gitHubBeforeAfter so that cleanup is done with a user logged in --- .../github/AbstractGitHubWireMockTest.java | 26 ++++++++++--------- src/test/java/org/kohsuke/github/AppTest.java | 6 ++--- .../github/GHContentIntegrationTest.java | 2 +- .../org/kohsuke/github/GHMilestoneTest.java | 2 +- .../kohsuke/github/GHOrganizationTest.java | 2 +- .../org/kohsuke/github/GHProjectCardTest.java | 6 ++--- .../kohsuke/github/GHProjectColumnTest.java | 4 +-- .../org/kohsuke/github/GHProjectTest.java | 2 +- .../org/kohsuke/github/GHPullRequestTest.java | 2 +- .../java/org/kohsuke/github/GHTagTest.java | 2 +- .../org/kohsuke/github/GHTreeBuilderTest.java | 2 +- .../github/WireMockStatusReporterTest.java | 4 +-- .../github/extras/GitHubCachingTest.java | 4 +-- .../github/extras/OkHttpConnectorTest.java | 4 +-- .../extras/okhttp3/GitHubCachingTest.java | 4 +-- .../extras/okhttp3/OkHttpConnectorTest.java | 4 +-- 16 files changed, 39 insertions(+), 37 deletions(-) diff --git a/src/test/java/org/kohsuke/github/AbstractGitHubWireMockTest.java b/src/test/java/org/kohsuke/github/AbstractGitHubWireMockTest.java index 579ea44aa..764e9dffb 100644 --- a/src/test/java/org/kohsuke/github/AbstractGitHubWireMockTest.java +++ b/src/test/java/org/kohsuke/github/AbstractGitHubWireMockTest.java @@ -38,11 +38,7 @@ public abstract class AbstractGitHubWireMockTest { */ protected GitHub gitHub; - /** - * {@link GitHub} instance for use before/after test. Traffic will not be part of snapshot when taken. Should only - * be used when isUseProxy() or isTakeSnapShot(). - */ - protected GitHub gitHubBeforeAfter; + private GitHub gitHubBeforeAfter; protected final String baseFilesClassPath = this.getClass().getName().replace('.', '/'); protected final String baseRecordPath = "src/test/resources/" + baseFilesClassPath + "/wiremock"; @@ -127,10 +123,10 @@ public abstract class AbstractGitHubWireMockTest { mockGitHub.isUseProxy()); } - protected void verifyAuthenticated() { + protected void verifyAuthenticated(GitHub instance) { assertThat( "GitHub connection believes it is anonymous. Make sure you set GITHUB_OAUTH or both GITHUB_USER and GITHUB_PASSWORD environment variables", - gitHub.isAnonymous(), + instance.isAnonymous(), is(false)); } @@ -172,12 +168,9 @@ public abstract class AbstractGitHubWireMockTest { String fullName = GITHUB_API_TEST_ORG + '/' + name; if (mockGitHub.isUseProxy()) { - // Needs to check if you are authenticated before doing this cleanup and repo creation - verifyAuthenticated(); - cleanupRepository(fullName); - GHRepository repository = gitHubBeforeAfter.getOrganization(GITHUB_API_TEST_ORG) + GHRepository repository = getGitHubBeforeAfter().getOrganization(GITHUB_API_TEST_ORG) .createRepository(name) .description("A test repository for testing the github-api project: " + name) .homepage("http://github-api.kohsuke.org/") @@ -211,7 +204,7 @@ public abstract class AbstractGitHubWireMockTest { if (mockGitHub.isUseProxy()) { tempGitHubRepositories.add(fullName); try { - GHRepository repository = gitHubBeforeAfter.getRepository(fullName); + GHRepository repository = getGitHubBeforeAfter().getRepository(fullName); if (repository != null) { repository.delete(); } @@ -222,6 +215,15 @@ public abstract class AbstractGitHubWireMockTest { } } + /** + * {@link GitHub} instance for use before/after test. Traffic will not be part of snapshot when taken. Should only + * be used when isUseProxy() or isTakeSnapShot(). + */ + public GitHub getGitHubBeforeAfter() { + verifyAuthenticated(gitHubBeforeAfter); + return gitHubBeforeAfter; + } + protected void kohsuke() { // No-op for now // Generally this means the test is doing something that requires additional access rights diff --git a/src/test/java/org/kohsuke/github/AppTest.java b/src/test/java/org/kohsuke/github/AppTest.java index cf3b50715..8ffefff1d 100755 --- a/src/test/java/org/kohsuke/github/AppTest.java +++ b/src/test/java/org/kohsuke/github/AppTest.java @@ -74,7 +74,7 @@ public class AppTest extends AbstractGitHubWireMockTest { private void cleanupUserRepository(final String name) throws IOException { if (mockGitHub.isUseProxy()) { - cleanupRepository(getUser(gitHubBeforeAfter).getLogin() + "/" + name); + cleanupRepository(getUser(getGitHubBeforeAfter()).getLogin() + "/" + name); } } @@ -437,7 +437,7 @@ public class AppTest extends AbstractGitHubWireMockTest { // System.out.println(hook); if (mockGitHub.isUseProxy()) { - r = gitHubBeforeAfter.getOrganization(GITHUB_API_TEST_ORG).getRepository("github-api"); + r = getGitHubBeforeAfter().getOrganization(GITHUB_API_TEST_ORG).getRepository("github-api"); for (GHHook h : r.getHooks()) { h.delete(); } @@ -826,7 +826,7 @@ public class AppTest extends AbstractGitHubWireMockTest { void cleanupLabel(String name) { if (mockGitHub.isUseProxy()) { try { - GHLabel t = gitHubBeforeAfter.getRepository("github-api-test-org/test-labels").getLabel("test"); + GHLabel t = getGitHubBeforeAfter().getRepository("github-api-test-org/test-labels").getLabel("test"); t.delete(); } catch (IOException e) { diff --git a/src/test/java/org/kohsuke/github/GHContentIntegrationTest.java b/src/test/java/org/kohsuke/github/GHContentIntegrationTest.java index 469d4f1fd..0fc101d1e 100644 --- a/src/test/java/org/kohsuke/github/GHContentIntegrationTest.java +++ b/src/test/java/org/kohsuke/github/GHContentIntegrationTest.java @@ -32,7 +32,7 @@ public class GHContentIntegrationTest extends AbstractGitHubWireMockTest { @After public void cleanup() throws Exception { if (mockGitHub.isUseProxy()) { - repo = gitHubBeforeAfter.getRepository("github-api-test-org/GHContentIntegrationTest"); + repo = getGitHubBeforeAfter().getRepository("github-api-test-org/GHContentIntegrationTest"); try { GHContent content = repo.getFileContent(createdFilename); if (content != null) { diff --git a/src/test/java/org/kohsuke/github/GHMilestoneTest.java b/src/test/java/org/kohsuke/github/GHMilestoneTest.java index 2fa76376b..4bdb79cdc 100644 --- a/src/test/java/org/kohsuke/github/GHMilestoneTest.java +++ b/src/test/java/org/kohsuke/github/GHMilestoneTest.java @@ -22,7 +22,7 @@ public class GHMilestoneTest extends AbstractGitHubWireMockTest { return; } - for (GHMilestone milestone : getRepository(gitHubBeforeAfter).listMilestones(GHIssueState.ALL)) { + for (GHMilestone milestone : getRepository(getGitHubBeforeAfter()).listMilestones(GHIssueState.ALL)) { if ("Original Title".equals(milestone.getTitle()) || "Updated Title".equals(milestone.getTitle())) { milestone.delete(); } diff --git a/src/test/java/org/kohsuke/github/GHOrganizationTest.java b/src/test/java/org/kohsuke/github/GHOrganizationTest.java index a07ae88e2..d451b7dbe 100644 --- a/src/test/java/org/kohsuke/github/GHOrganizationTest.java +++ b/src/test/java/org/kohsuke/github/GHOrganizationTest.java @@ -23,7 +23,7 @@ public class GHOrganizationTest extends AbstractGitHubWireMockTest { return; } - GHTeam team = gitHubBeforeAfter.getOrganization(GITHUB_API_TEST_ORG).getTeamByName(TEAM_NAME_CREATE); + GHTeam team = getGitHubBeforeAfter().getOrganization(GITHUB_API_TEST_ORG).getTeamByName(TEAM_NAME_CREATE); if (team != null) { team.delete(); } diff --git a/src/test/java/org/kohsuke/github/GHProjectCardTest.java b/src/test/java/org/kohsuke/github/GHProjectCardTest.java index e6bf0b06b..b2f00e622 100644 --- a/src/test/java/org/kohsuke/github/GHProjectCardTest.java +++ b/src/test/java/org/kohsuke/github/GHProjectCardTest.java @@ -74,7 +74,7 @@ public class GHProjectCardTest extends AbstractGitHubWireMockTest { public void after() throws IOException { if (mockGitHub.isUseProxy()) { if (card != null) { - card = gitHubBeforeAfter.getProjectCard(card.getId()); + card = getGitHubBeforeAfter().getProjectCard(card.getId()); try { card.delete(); card = null; @@ -83,7 +83,7 @@ public class GHProjectCardTest extends AbstractGitHubWireMockTest { } } if (column != null) { - column = gitHubBeforeAfter.getProjectColumn(column.getId()); + column = getGitHubBeforeAfter().getProjectColumn(column.getId()); try { column.delete(); column = null; @@ -92,7 +92,7 @@ public class GHProjectCardTest extends AbstractGitHubWireMockTest { } } if (project != null) { - project = gitHubBeforeAfter.getProject(project.getId()); + project = getGitHubBeforeAfter().getProject(project.getId()); try { project.delete(); project = null; diff --git a/src/test/java/org/kohsuke/github/GHProjectColumnTest.java b/src/test/java/org/kohsuke/github/GHProjectColumnTest.java index 29c7e0fd8..0973eeb32 100644 --- a/src/test/java/org/kohsuke/github/GHProjectColumnTest.java +++ b/src/test/java/org/kohsuke/github/GHProjectColumnTest.java @@ -48,7 +48,7 @@ public class GHProjectColumnTest extends AbstractGitHubWireMockTest { public void after() throws IOException { if (mockGitHub.isUseProxy()) { if (column != null) { - column = gitHubBeforeAfter.getProjectColumn(column.getId()); + column = getGitHubBeforeAfter().getProjectColumn(column.getId()); try { column.delete(); column = null; @@ -57,7 +57,7 @@ public class GHProjectColumnTest extends AbstractGitHubWireMockTest { } } if (project != null) { - project = gitHubBeforeAfter.getProject(project.getId()); + project = getGitHubBeforeAfter().getProject(project.getId()); try { project.delete(); project = null; diff --git a/src/test/java/org/kohsuke/github/GHProjectTest.java b/src/test/java/org/kohsuke/github/GHProjectTest.java index 178a1d2b5..cc10b1fb8 100644 --- a/src/test/java/org/kohsuke/github/GHProjectTest.java +++ b/src/test/java/org/kohsuke/github/GHProjectTest.java @@ -69,7 +69,7 @@ public class GHProjectTest extends AbstractGitHubWireMockTest { public void after() throws IOException { if (mockGitHub.isUseProxy()) { if (project != null) { - project = gitHubBeforeAfter.getProject(project.getId()); + project = getGitHubBeforeAfter().getProject(project.getId()); try { project.delete(); project = null; diff --git a/src/test/java/org/kohsuke/github/GHPullRequestTest.java b/src/test/java/org/kohsuke/github/GHPullRequestTest.java index 0805f1fc5..4b9a8da2d 100644 --- a/src/test/java/org/kohsuke/github/GHPullRequestTest.java +++ b/src/test/java/org/kohsuke/github/GHPullRequestTest.java @@ -30,7 +30,7 @@ public class GHPullRequestTest extends AbstractGitHubWireMockTest { return; } - for (GHPullRequest pr : getRepository(this.gitHubBeforeAfter).getPullRequests(GHIssueState.OPEN)) { + for (GHPullRequest pr : getRepository(this.getGitHubBeforeAfter()).getPullRequests(GHIssueState.OPEN)) { pr.close(); } } diff --git a/src/test/java/org/kohsuke/github/GHTagTest.java b/src/test/java/org/kohsuke/github/GHTagTest.java index 186740b6a..5aac4d083 100644 --- a/src/test/java/org/kohsuke/github/GHTagTest.java +++ b/src/test/java/org/kohsuke/github/GHTagTest.java @@ -24,7 +24,7 @@ public class GHTagTest extends AbstractGitHubWireMockTest { } try { - GHRef ref = getRepository(this.gitHubBeforeAfter).getRef("tags/create_tag_test"); + GHRef ref = getRepository(this.getGitHubBeforeAfter()).getRef("tags/create_tag_test"); if (ref != null) { ref.delete(); } diff --git a/src/test/java/org/kohsuke/github/GHTreeBuilderTest.java b/src/test/java/org/kohsuke/github/GHTreeBuilderTest.java index df948e430..b2bb175dc 100644 --- a/src/test/java/org/kohsuke/github/GHTreeBuilderTest.java +++ b/src/test/java/org/kohsuke/github/GHTreeBuilderTest.java @@ -33,7 +33,7 @@ public class GHTreeBuilderTest extends AbstractGitHubWireMockTest { @After public void cleanup() throws Exception { if (mockGitHub.isUseProxy()) { - repo = gitHubBeforeAfter.getRepository(REPO_NAME); + repo = getGitHubBeforeAfter().getRepository(REPO_NAME); Arrays.asList(PATH_SCRIPT, PATH_README, PATH_DATA1, PATH_DATA2).forEach(path -> { try { GHContent content = repo.getFileContent(path); diff --git a/src/test/java/org/kohsuke/github/WireMockStatusReporterTest.java b/src/test/java/org/kohsuke/github/WireMockStatusReporterTest.java index 2a2babd36..554f5d7b6 100644 --- a/src/test/java/org/kohsuke/github/WireMockStatusReporterTest.java +++ b/src/test/java/org/kohsuke/github/WireMockStatusReporterTest.java @@ -25,7 +25,7 @@ public class WireMockStatusReporterTest extends AbstractGitHubWireMockTest { snapshotNotAllowed(); requireProxy("Tests proper configuration when proxying."); - verifyAuthenticated(); + verifyAuthenticated(gitHub); assertThat(gitHub.login, not(equalTo(STUBBED_USER_LOGIN))); @@ -47,7 +47,7 @@ public class WireMockStatusReporterTest extends AbstractGitHubWireMockTest { assumeFalse("Test only valid when not proxying", mockGitHub.isUseProxy()); - verifyAuthenticated(); + verifyAuthenticated(gitHub); assertThat(gitHub.login, equalTo(STUBBED_USER_LOGIN)); GHUser user = gitHub.getMyself(); diff --git a/src/test/java/org/kohsuke/github/extras/GitHubCachingTest.java b/src/test/java/org/kohsuke/github/extras/GitHubCachingTest.java index 837be05f5..97e4842cd 100644 --- a/src/test/java/org/kohsuke/github/extras/GitHubCachingTest.java +++ b/src/test/java/org/kohsuke/github/extras/GitHubCachingTest.java @@ -43,11 +43,11 @@ public class GitHubCachingTest extends AbstractGitHubWireMockTest { @Before public void setupRepo() throws Exception { if (mockGitHub.isUseProxy()) { - for (GHPullRequest pr : getRepository(this.gitHubBeforeAfter).getPullRequests(GHIssueState.OPEN)) { + for (GHPullRequest pr : getRepository(this.getGitHubBeforeAfter()).getPullRequests(GHIssueState.OPEN)) { pr.close(); } try { - GHRef ref = getRepository(this.gitHubBeforeAfter).getRef(testRefName); + GHRef ref = getRepository(this.getGitHubBeforeAfter()).getRef(testRefName); ref.delete(); } catch (IOException e) { } diff --git a/src/test/java/org/kohsuke/github/extras/OkHttpConnectorTest.java b/src/test/java/org/kohsuke/github/extras/OkHttpConnectorTest.java index 162753ec9..2f940b543 100644 --- a/src/test/java/org/kohsuke/github/extras/OkHttpConnectorTest.java +++ b/src/test/java/org/kohsuke/github/extras/OkHttpConnectorTest.java @@ -75,7 +75,7 @@ public class OkHttpConnectorTest extends AbstractGitHubWireMockTest { @Before public void setupRepo() throws Exception { if (mockGitHub.isUseProxy()) { - GHRepository repo = getRepository(gitHubBeforeAfter); + GHRepository repo = getRepository(getGitHubBeforeAfter()); repo.setDescription("Resetting"); // Let things settle a bit between tests when working against the live site @@ -254,7 +254,7 @@ public class OkHttpConnectorTest extends AbstractGitHubWireMockTest { // Get Tricky - make a change via a different client if (mockGitHub.isUseProxy()) { - GHRepository altRepo = getRepository(gitHubBeforeAfter); + GHRepository altRepo = getRepository(getGitHubBeforeAfter()); altRepo.setDescription("Tricky"); } diff --git a/src/test/java/org/kohsuke/github/extras/okhttp3/GitHubCachingTest.java b/src/test/java/org/kohsuke/github/extras/okhttp3/GitHubCachingTest.java index 0ef5cd3e4..ed42ca71f 100644 --- a/src/test/java/org/kohsuke/github/extras/okhttp3/GitHubCachingTest.java +++ b/src/test/java/org/kohsuke/github/extras/okhttp3/GitHubCachingTest.java @@ -44,11 +44,11 @@ public class GitHubCachingTest extends AbstractGitHubWireMockTest { @Before public void setupRepo() throws Exception { if (mockGitHub.isUseProxy()) { - for (GHPullRequest pr : getRepository(this.gitHubBeforeAfter).getPullRequests(GHIssueState.OPEN)) { + for (GHPullRequest pr : getRepository(this.getGitHubBeforeAfter()).getPullRequests(GHIssueState.OPEN)) { pr.close(); } try { - GHRef ref = getRepository(this.gitHubBeforeAfter).getRef(testRefName); + GHRef ref = getRepository(this.getGitHubBeforeAfter()).getRef(testRefName); ref.delete(); } catch (IOException e) { } diff --git a/src/test/java/org/kohsuke/github/extras/okhttp3/OkHttpConnectorTest.java b/src/test/java/org/kohsuke/github/extras/okhttp3/OkHttpConnectorTest.java index d67ff562f..dea66e4b8 100644 --- a/src/test/java/org/kohsuke/github/extras/okhttp3/OkHttpConnectorTest.java +++ b/src/test/java/org/kohsuke/github/extras/okhttp3/OkHttpConnectorTest.java @@ -80,7 +80,7 @@ public class OkHttpConnectorTest extends AbstractGitHubWireMockTest { @Before public void setupRepo() throws Exception { if (mockGitHub.isUseProxy()) { - GHRepository repo = getRepository(gitHubBeforeAfter); + GHRepository repo = getRepository(getGitHubBeforeAfter()); repo.setDescription("Resetting"); // Let things settle a bit between tests when working against the live site @@ -262,7 +262,7 @@ public class OkHttpConnectorTest extends AbstractGitHubWireMockTest { // Get Tricky - make a change via a different client if (mockGitHub.isUseProxy()) { - GHRepository altRepo = getRepository(gitHubBeforeAfter); + GHRepository altRepo = getRepository(getGitHubBeforeAfter()); altRepo.setDescription("Tricky"); }