From d35f206bd97ffce79b3db2b9e392f322a08377fb Mon Sep 17 00:00:00 2001 From: Marc Salles Date: Wed, 13 Mar 2019 16:16:51 +0100 Subject: [PATCH 01/10] Fixes folder credentials not working for Slack --- .../jenkins/plugins/slack/SlackNotifier.java | 13 +- .../plugins/slack/StandardSlackService.java | 23 ++-- .../plugins/slack/workflow/SlackSendStep.java | 8 +- .../slack/CloseableHttpClientStub.java | 6 + .../plugins/slack/SlackNotifierStub.java | 4 +- .../plugins/slack/SlackNotifierTest.java | 2 +- .../StandardSlackServiceCredentialsTest.java | 127 ++++++++++++++++++ .../slack/StandardSlackServiceStub.java | 6 +- .../slack/StandardSlackServiceTest.java | 37 +++-- .../slack/workflow/SlackSendStepTest.java | 26 ++-- 10 files changed, 206 insertions(+), 46 deletions(-) create mode 100644 src/test/java/jenkins/plugins/slack/StandardSlackServiceCredentialsTest.java diff --git a/src/main/java/jenkins/plugins/slack/SlackNotifier.java b/src/main/java/jenkins/plugins/slack/SlackNotifier.java index f39c7345..ae65490a 100755 --- a/src/main/java/jenkins/plugins/slack/SlackNotifier.java +++ b/src/main/java/jenkins/plugins/slack/SlackNotifier.java @@ -10,6 +10,7 @@ import hudson.model.AbstractProject; import hudson.model.BuildListener; import hudson.model.Item; +import hudson.model.Project; import hudson.security.ACL; import hudson.tasks.BuildStepDescriptor; import hudson.tasks.BuildStepMonitor; @@ -427,8 +428,7 @@ public SlackService newSlackService(AbstractBuild r, BuildListener listener) { authToken = env.expand(authToken); authTokenCredentialId = env.expand(authTokenCredentialId); room = env.expand(room); - - return new StandardSlackService(baseUrl, teamDomain, authToken, authTokenCredentialId, botUser, room); + return new StandardSlackService(baseUrl, teamDomain, authToken, authTokenCredentialId, botUser, room, r.getProject()); } @Override @@ -604,8 +604,8 @@ public boolean configure(StaplerRequest req, JSONObject formData) { return true; } - SlackService getSlackService(final String baseUrl, final String teamDomain, final String authTokenCredentialId, final boolean botUser, final String room) { - return new StandardSlackService(baseUrl, teamDomain, authTokenCredentialId, botUser, room); + SlackService getSlackService(final String baseUrl, final String teamDomain, final String authTokenCredentialId, final boolean botUser, final String room, final Item project) { + return new StandardSlackService(baseUrl, teamDomain, authTokenCredentialId, botUser, room, project); } @Nonnull @@ -618,7 +618,8 @@ public FormValidation doTestConnection(@QueryParameter("baseUrl") final String b @QueryParameter("teamDomain") final String teamDomain, @QueryParameter("tokenCredentialId") final String tokenCredentialId, @QueryParameter("botUser") final boolean botUser, - @QueryParameter("room") final String room) { + @QueryParameter("room") final String room, + @AncestorInPath Project project) { try { String targetUrl = baseUrl; @@ -636,7 +637,7 @@ public FormValidation doTestConnection(@QueryParameter("baseUrl") final String b this.tokenCredentialId; String targetRoom = Util.fixEmpty(room) != null ? room : this.room; - SlackService testSlackService = getSlackService(targetUrl, targetDomain, targetTokenCredentialId, targetBotUser, targetRoom); + SlackService testSlackService = getSlackService(targetUrl, targetDomain, targetTokenCredentialId, targetBotUser, targetRoom, project); String message = "Slack/Jenkins plugin: you're all set on " + DisplayURLProvider.get().getRoot(); boolean success = testSlackService.publish(message, "good"); return success ? FormValidation.ok("Success") : FormValidation.error("Failure"); diff --git a/src/main/java/jenkins/plugins/slack/StandardSlackService.java b/src/main/java/jenkins/plugins/slack/StandardSlackService.java index 0edd2eb1..e668bab9 100755 --- a/src/main/java/jenkins/plugins/slack/StandardSlackService.java +++ b/src/main/java/jenkins/plugins/slack/StandardSlackService.java @@ -3,6 +3,7 @@ import com.cloudbees.plugins.credentials.CredentialsMatcher; import com.cloudbees.plugins.credentials.CredentialsMatchers; import hudson.ProxyConfiguration; +import hudson.model.Item; import hudson.security.ACL; import jenkins.model.Jenkins; import jenkins.plugins.slack.logging.BuildKey; @@ -50,16 +51,17 @@ public class StandardSlackService implements SlackService { private String[] roomIds; private boolean replyBroadcast; private String responseString = null; + private Item item; - public StandardSlackService(String baseUrl, String teamDomain, String authTokenCredentialId, boolean botUser, String roomId) { - this(baseUrl, teamDomain, null, authTokenCredentialId, botUser, roomId, false); + public StandardSlackService(String baseUrl, String teamDomain, String authTokenCredentialId, boolean botUser, String roomId, Item item) { + this(baseUrl, teamDomain, null, authTokenCredentialId, botUser, roomId, false, item); } - public StandardSlackService(String baseUrl, String teamDomain, String token, String authTokenCredentialId, boolean botUser, String roomId) { - this(baseUrl, teamDomain, token, authTokenCredentialId, botUser, roomId, false); + public StandardSlackService(String baseUrl, String teamDomain, String token, String authTokenCredentialId, boolean botUser, String roomId, Item item) { + this(baseUrl, teamDomain, token, authTokenCredentialId, botUser, roomId, false, item); } - public StandardSlackService(String baseUrl, String teamDomain, String token, String authTokenCredentialId, boolean botUser, String roomId, boolean replyBroadcast) { + public StandardSlackService(String baseUrl, String teamDomain, String token, String authTokenCredentialId, boolean botUser, String roomId, boolean replyBroadcast, Item item) { super(); this.baseUrl = baseUrl; if(this.baseUrl != null && !this.baseUrl.isEmpty() && !this.baseUrl.endsWith("/")) { @@ -71,6 +73,7 @@ public StandardSlackService(String baseUrl, String teamDomain, String token, Str this.botUser = botUser; this.roomIds = roomId.split("[,; ]+"); this.replyBroadcast = replyBroadcast; + this.item = item; } public String getResponseString() { @@ -120,12 +123,12 @@ public boolean publish(String message, JSONArray attachments, String color) { roomId = splitThread[0]; threadTs = splitThread[1]; } - + final String token = getTokenToUse(); //prepare post methods for both requests types if (!botUser || !StringUtils.isEmpty(baseUrl)) { - url = "https://" + teamDomain + "." + host + "/services/hooks/jenkins-ci?token=" + getTokenToUse(); + url = "https://" + teamDomain + "." + host + "/services/hooks/jenkins-ci?token=" + token; if (!StringUtils.isEmpty(baseUrl)) { - url = baseUrl + getTokenToUse(); + url = baseUrl + token; } post = new HttpPost(url); JSONObject json = new JSONObject(); @@ -139,7 +142,7 @@ public boolean publish(String message, JSONArray attachments, String color) { nvps.add(new BasicNameValuePair("payload", json.toString())); } else { - url = "https://slack.com/api/chat.postMessage?token=" + getTokenToUse() + + url = "https://slack.com/api/chat.postMessage?token=" + token + "&channel=" + roomId.replace("#", "") + "&link_names=1" + "&as_user=true"; @@ -200,7 +203,7 @@ private String getTokenToUse() { } private StringCredentials lookupCredentials(String credentialId) { - List credentials = com.cloudbees.plugins.credentials.CredentialsProvider.lookupCredentials(StringCredentials.class, Jenkins.get(), ACL.SYSTEM, Collections.emptyList()); + List credentials = com.cloudbees.plugins.credentials.CredentialsProvider.lookupCredentials(StringCredentials.class, item, ACL.SYSTEM, Collections.emptyList()); CredentialsMatcher matcher = CredentialsMatchers.withId(credentialId); return CredentialsMatchers.firstOrNull(credentials, matcher); } diff --git a/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java b/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java index ae92bf5d..85124263 100644 --- a/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java +++ b/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java @@ -232,7 +232,7 @@ public static class SlackSendStepExecution extends SynchronousNonBlockingStepExe protected SlackResponse run() throws Exception { Jenkins jenkins = Jenkins.get(); - + Project project = getContext().get(Project.class); SlackNotifier.DescriptorImpl slackDesc = jenkins.getDescriptorByType(SlackNotifier.DescriptorImpl.class); String baseUrl = step.baseUrl != null ? step.baseUrl : slackDesc.getBaseUrl(); @@ -253,7 +253,7 @@ protected SlackResponse run() throws Exception { ); SlackService slackService = getSlackService( - baseUrl, teamDomain, token, tokenCredentialId, botUser, channel, step.replyBroadcast + baseUrl, teamDomain, token, tokenCredentialId, botUser, channel, step.replyBroadcast, project ); final boolean publishSuccess; if (step.attachments != null) { @@ -328,8 +328,8 @@ private String defaultIfEmpty(String value) { } //streamline unit testing - SlackService getSlackService(String baseUrl, String team, String token, String tokenCredentialId, boolean botUser, String channel, boolean replyBroadcast) { - return new StandardSlackService(baseUrl, team, token, tokenCredentialId, botUser, channel, replyBroadcast); + SlackService getSlackService(String baseUrl, String team, String token, String tokenCredentialId, boolean botUser, String channel, boolean replyBroadcast, Item item) { + return new StandardSlackService(baseUrl, team, token, tokenCredentialId, botUser, channel, replyBroadcast, item); } } } diff --git a/src/test/java/jenkins/plugins/slack/CloseableHttpClientStub.java b/src/test/java/jenkins/plugins/slack/CloseableHttpClientStub.java index 4ea06df6..298fde83 100644 --- a/src/test/java/jenkins/plugins/slack/CloseableHttpClientStub.java +++ b/src/test/java/jenkins/plugins/slack/CloseableHttpClientStub.java @@ -16,8 +16,10 @@ public class CloseableHttpClientStub extends CloseableHttpClient { private int numberOfCallsToExecuteMethod; private int httpStatus; private boolean failAlternateResponses = false; + private HttpUriRequest lastRequest = null; public CloseableHttpResponse execute(HttpUriRequest post) { + lastRequest = post; numberOfCallsToExecuteMethod++; if (failAlternateResponses && (numberOfCallsToExecuteMethod % 2 == 0)) { return new CloseableHttpResponseStub(HttpStatus.SC_NOT_FOUND); @@ -57,4 +59,8 @@ public void setHttpStatus(int httpStatus) { public void setFailAlternateResponses(boolean failAlternateResponses) { this.failAlternateResponses = failAlternateResponses; } + + public HttpUriRequest getLastRequest() { + return lastRequest; + } } diff --git a/src/test/java/jenkins/plugins/slack/SlackNotifierStub.java b/src/test/java/jenkins/plugins/slack/SlackNotifierStub.java index 440625d0..ef2322a4 100644 --- a/src/test/java/jenkins/plugins/slack/SlackNotifierStub.java +++ b/src/test/java/jenkins/plugins/slack/SlackNotifierStub.java @@ -1,5 +1,7 @@ package jenkins.plugins.slack; +import hudson.model.Item; + public class SlackNotifierStub extends SlackNotifier { public SlackNotifierStub(String baseUrl, String teamDomain, String authToken, boolean botUser, String room, String authTokenCredentialId, @@ -23,7 +25,7 @@ public synchronized void load() { } @Override - SlackService getSlackService(final String baseUrl, final String teamDomain, final String authTokenCredentialId, final boolean botUser, final String room) { + SlackService getSlackService(final String baseUrl, final String teamDomain, final String authTokenCredentialId, final boolean botUser, final String room, final Item project) { return slackService; } diff --git a/src/test/java/jenkins/plugins/slack/SlackNotifierTest.java b/src/test/java/jenkins/plugins/slack/SlackNotifierTest.java index 2a9070bd..8ec62b46 100644 --- a/src/test/java/jenkins/plugins/slack/SlackNotifierTest.java +++ b/src/test/java/jenkins/plugins/slack/SlackNotifierTest.java @@ -52,7 +52,7 @@ public void testDoTestConnection() { } descriptor.setSlackService(slackServiceStub); FormValidation result = descriptor - .doTestConnection("baseUrl", "teamDomain", "authTokenCredentialId", false, "room"); + .doTestConnection("baseUrl", "teamDomain", "authTokenCredentialId", false, "room", null); assertEquals(result.kind, expectedResult); } diff --git a/src/test/java/jenkins/plugins/slack/StandardSlackServiceCredentialsTest.java b/src/test/java/jenkins/plugins/slack/StandardSlackServiceCredentialsTest.java new file mode 100644 index 00000000..eb241d3e --- /dev/null +++ b/src/test/java/jenkins/plugins/slack/StandardSlackServiceCredentialsTest.java @@ -0,0 +1,127 @@ +package jenkins.plugins.slack; + +import com.cloudbees.plugins.credentials.CredentialsProvider; +import hudson.ExtensionList; +import hudson.model.*; +import hudson.security.ACL; +import hudson.util.Secret; +import jenkins.model.Jenkins; +import org.apache.http.HttpStatus; +import org.jenkinsci.plugins.plaincredentials.StringCredentials; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Matchers; +import org.mockito.Mock; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.powermock.api.mockito.PowerMockito.mock; +import static org.powermock.api.mockito.PowerMockito.when; + +@RunWith(PowerMockRunner.class) +@PrepareForTest({Jenkins.class, StandardSlackService.class, CredentialsProvider.class, Secret.class}) +public class StandardSlackServiceCredentialsTest { + @Mock + Jenkins jenkins; + + @Before + public void setUp() { + PowerMockito.mockStatic(Jenkins.class); + } + + @Test + public void providedJobCredentialsAreUsed() { + final String id = "cred-id"; + final String secretText = "secret-text"; + final Job job = createJobWithAvailableCredentialId(id, secretText); + final StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", null, id, true, "#room1:1528317530", job); + final CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); + httpClientStub.setHttpStatus(HttpStatus.SC_OK); + service.setHttpClient(httpClientStub); + service.publish("message"); + assertTrue(httpClientStub.getLastRequest().getURI().toString().contains(secretText)); + } + + @Test + public void globalCredentialsCanBeUsed() { + final String id = "cred-2id"; + final String secretText = "secret-2text"; + setupGlobalAvailableCredentialId(id, secretText); + final StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", null, id, true, "#room1:1528317530", null); + final CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); + httpClientStub.setHttpStatus(HttpStatus.SC_OK); + service.setHttpClient(httpClientStub); + service.publish("message"); + assertTrue(httpClientStub.getLastRequest().getURI().toString().contains(secretText)); + } + + @Test + public void otherJobCredentialsAreNotObtained() { + final String id = "cred-id"; + final String secretText = "secret-text"; + final Job job = createJobWithAvailableCredentialId(id, secretText); + final StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", null, "wrongid", true, "#room1:1528317530", job); + final CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); + httpClientStub.setHttpStatus(HttpStatus.SC_OK); + service.setHttpClient(httpClientStub); + service.publish("message"); + assertFalse(httpClientStub.getLastRequest().getURI().toString().contains(secretText)); + } + + @Test + public void ifNoJobProvidedTokenIsUsed() { + final String token = "explicittoken"; + final StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", token, null, true, "#room1:1528317530", null); + final CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); + httpClientStub.setHttpStatus(HttpStatus.SC_OK); + service.setHttpClient(httpClientStub); + service.publish("message"); + assertTrue(httpClientStub.getLastRequest().getURI().toString().contains(token)); + } + + /** + * Creates a mocked job and sets up the mocking mechanism so the provided credential is obtainable. + * @param id the id of the credential to use. + * @param secretText the secret text of the credential. + * @return a mocked job set up so it can use the provided credentials + */ + private Job createJobWithAvailableCredentialId(final String id, final String secretText) { + final Job job = mock(Job.class); + final CredentialsProvider credentialsProvider = mock(CredentialsProvider.class); + final List credentialList = setupMocks(id, secretText, credentialsProvider); + when(credentialsProvider.getCredentials(Matchers.eq(StringCredentials.class), Matchers.eq(job), Matchers.eq(ACL.SYSTEM), Matchers.eq(Collections.emptyList()))).thenReturn(credentialList); + return job; + } + + private void setupGlobalAvailableCredentialId(final String id, final String secretText) { + final CredentialsProvider credentialsProvider = mock(CredentialsProvider.class); + final List credentialList = setupMocks(id, secretText, credentialsProvider); + when(credentialsProvider.getCredentials(Matchers.eq(StringCredentials.class), Matchers.eq(jenkins), Matchers.eq(ACL.SYSTEM), Matchers.eq(Collections.emptyList()))).thenReturn(credentialList); + } + + private List setupMocks(String id, String secretText, CredentialsProvider credentialsProvider) { + final ExtensionList extensionList = mock(ExtensionList.class); + final List listProviders = new ArrayList<>(1); + final List credentialList = new ArrayList<>(); + final StringCredentials credentials = mock(StringCredentials.class); + final Secret secret = mock(Secret.class); + listProviders.add(credentialsProvider); + when(extensionList.iterator()).thenReturn(listProviders.iterator()); + when(Jenkins.getInstance()).thenReturn(jenkins); + when(jenkins.getExtensionList(CredentialsProvider.class)).thenReturn(extensionList); + when(credentials.getId()).thenReturn(id); + when(secret.getPlainText()).thenReturn(secretText); + when(credentials.getSecret()).thenReturn(secret); + credentialList.add(credentials); + return credentialList; + } + +} diff --git a/src/test/java/jenkins/plugins/slack/StandardSlackServiceStub.java b/src/test/java/jenkins/plugins/slack/StandardSlackServiceStub.java index bf2763f5..89f6c160 100644 --- a/src/test/java/jenkins/plugins/slack/StandardSlackServiceStub.java +++ b/src/test/java/jenkins/plugins/slack/StandardSlackServiceStub.java @@ -1,11 +1,13 @@ package jenkins.plugins.slack; +import hudson.model.Item; + public class StandardSlackServiceStub extends StandardSlackService { private CloseableHttpClientStub httpClientStub; - public StandardSlackServiceStub(String baseUrl, String teamDomain, String token, String tokenCredentialId, boolean botUser, String roomId) { - super(baseUrl, teamDomain, token, tokenCredentialId, botUser, roomId); + public StandardSlackServiceStub(String baseUrl, String teamDomain, String token, String tokenCredentialId, boolean botUser, String roomId, Item item) { + super(baseUrl, teamDomain, token, tokenCredentialId, botUser, roomId, item); } @Override diff --git a/src/test/java/jenkins/plugins/slack/StandardSlackServiceTest.java b/src/test/java/jenkins/plugins/slack/StandardSlackServiceTest.java index 402cc6ea..b3cb6506 100755 --- a/src/test/java/jenkins/plugins/slack/StandardSlackServiceTest.java +++ b/src/test/java/jenkins/plugins/slack/StandardSlackServiceTest.java @@ -1,11 +1,13 @@ package jenkins.plugins.slack; +import hudson.model.Job; import org.apache.http.HttpStatus; import org.junit.Test; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; public class StandardSlackServiceTest { /** @@ -13,7 +15,7 @@ public class StandardSlackServiceTest { */ @Test public void publishWithBadHostShouldNotRethrowExceptions() { - StandardSlackService service = new StandardSlackService("", "foo", "token", null, false, "#general"); + StandardSlackService service = new StandardSlackService("", "foo", "token", null, false, "#general", null); service.setHost("hostvaluethatwillcausepublishtofail"); service.publish("message"); } @@ -23,7 +25,7 @@ public void publishWithBadHostShouldNotRethrowExceptions() { */ @Test public void invalidTeamDomainShouldFail() { - StandardSlackService service = new StandardSlackService("", "my", "token", null, false, "#general"); + StandardSlackService service = new StandardSlackService("", "my", "token", null, false, "#general", null); service.publish("message"); } @@ -32,13 +34,13 @@ public void invalidTeamDomainShouldFail() { */ @Test public void invalidTokenShouldFail() { - StandardSlackService service = new StandardSlackService("", "tinyspeck", "token", null, false, "#general"); + StandardSlackService service = new StandardSlackService("", "tinyspeck", "token", null, false, "#general", null); service.publish("message"); } @Test public void publishToASingleRoomSendsASingleMessage() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1"); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1", null); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); service.setHttpClient(httpClientStub); service.publish("message"); @@ -47,7 +49,7 @@ public void publishToASingleRoomSendsASingleMessage() { @Test public void publishToMultipleRoomsSendsAMessageToEveryRoom() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1,#room2,#room3"); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1,#room2,#room3", null); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); service.setHttpClient(httpClientStub); service.publish("message"); @@ -56,7 +58,18 @@ public void publishToMultipleRoomsSendsAMessageToEveryRoom() { @Test public void successfulPublishToASingleRoomReturnsTrue() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1"); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1", null); + CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); + httpClientStub.setHttpStatus(HttpStatus.SC_OK); + service.setHttpClient(httpClientStub); + assertTrue(service.publish("message")); + } + + + @Test + public void successfulPublishToSingleRoomWithProvidedJobReturnsTrue() { + Job job = mock(Job.class); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1", job); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); @@ -65,7 +78,7 @@ public void successfulPublishToASingleRoomReturnsTrue() { @Test public void successfulPublishToMultipleRoomsReturnsTrue() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1,#room2,#room3"); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1,#room2,#room3", null); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); @@ -74,7 +87,7 @@ public void successfulPublishToMultipleRoomsReturnsTrue() { @Test public void failedPublishToASingleRoomReturnsFalse() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1"); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1", null); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_NOT_FOUND); service.setHttpClient(httpClientStub); @@ -83,7 +96,7 @@ public void failedPublishToASingleRoomReturnsFalse() { @Test public void singleFailedPublishToMultipleRoomsReturnsFalse() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1,#room2,#room3"); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1,#room2,#room3", null); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setFailAlternateResponses(true); httpClientStub.setHttpStatus(HttpStatus.SC_OK); @@ -93,7 +106,7 @@ public void singleFailedPublishToMultipleRoomsReturnsFalse() { @Test public void publishToEmptyRoomReturnsTrue() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, ""); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "", null); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); @@ -103,7 +116,7 @@ public void publishToEmptyRoomReturnsTrue() { @Test public void sendAsBotUserReturnsTrue() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, true, "#room1"); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, true, "#room1", null); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); @@ -112,7 +125,7 @@ public void sendAsBotUserReturnsTrue() { @Test public void sendAsBotUserInThreadReturnsTrue() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, true, "#room1:1528317530"); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, true, "#room1:1528317530", null); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); diff --git a/src/test/java/jenkins/plugins/slack/workflow/SlackSendStepTest.java b/src/test/java/jenkins/plugins/slack/workflow/SlackSendStepTest.java index f8b7e7e1..c54c8d5a 100644 --- a/src/test/java/jenkins/plugins/slack/workflow/SlackSendStepTest.java +++ b/src/test/java/jenkins/plugins/slack/workflow/SlackSendStepTest.java @@ -1,5 +1,7 @@ package jenkins.plugins.slack.workflow; +import hudson.model.Item; +import hudson.model.Project; import hudson.model.TaskListener; import jenkins.model.Jenkins; import jenkins.plugins.slack.SlackNotifier; @@ -25,6 +27,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.anyBoolean; import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.doNothing; @@ -47,6 +50,8 @@ public class SlackSendStepTest { @Mock StepContext stepContextMock; @Mock + Project project; + @Mock SlackService slackServiceMock; @Mock Jenkins jenkins; @@ -59,6 +64,7 @@ public void setUp() throws IOException, InterruptedException { when(jenkins.getDescriptorByType(SlackNotifier.DescriptorImpl.class)).thenReturn(slackDescMock); when(taskListenerMock.getLogger()).thenReturn(printStreamMock); when(stepContextMock.get(TaskListener.class)).thenReturn(taskListenerMock); + when(stepContextMock.get(Project.class)).thenReturn(project); } @Test @@ -82,11 +88,11 @@ public void testStepOverrides() throws Exception { when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean())).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), any(Item.class))).thenReturn(slackServiceMock); when(slackServiceMock.publish(anyString(), anyString())).thenReturn(true); stepExecution.run(); - verify(stepExecution, times(1)).getSlackService("baseUrl/", "teamDomain", "token", "tokenCredentialId", true, "channel", false); + verify(stepExecution, times(1)).getSlackService("baseUrl/", "teamDomain", "token", "tokenCredentialId", true, "channel", false, project); verify(slackServiceMock, times(1)).publish("message", "good"); } @@ -109,7 +115,7 @@ public void testStepWithAttachments() throws Exception { when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean())).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), any(Item.class))).thenReturn(slackServiceMock); stepExecution.run(); verify(slackServiceMock, times(0)).publish("message", ""); @@ -167,10 +173,10 @@ public void testValuesForGlobalConfig() throws Exception { when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean())).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), any(Item.class))).thenReturn(slackServiceMock); stepExecution.run(); - verify(stepExecution, times(1)).getSlackService("globalBaseUrl", "globalTeamDomain", null, "globalTokenCredentialId", false, "globalChannel", false); + verify(stepExecution, times(1)).getSlackService("globalBaseUrl", "globalTeamDomain", null, "globalTokenCredentialId", false, "globalChannel", false, project); verify(slackServiceMock, times(1)).publish("message", ""); } @@ -193,10 +199,10 @@ public void testReplyBroadcast() throws Exception { when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean())).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), any(Item.class))).thenReturn(slackServiceMock); stepExecution.run(); - verify(stepExecution, times(1)).getSlackService("globalBaseUrl", "globalTeamDomain", null, "globalTokenCredentialId", false, "globalChannel", true); + verify(stepExecution, times(1)).getSlackService("globalBaseUrl", "globalTeamDomain", null, "globalTokenCredentialId", false, "globalChannel", true, project); verify(slackServiceMock, times(1)).publish("message", ""); } @@ -212,7 +218,7 @@ public void testNonNullEmptyColor() throws Exception { when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean())).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), any(Item.class))).thenReturn(slackServiceMock); stepExecution.run(); verify(slackServiceMock, times(1)).publish("message", ""); @@ -237,7 +243,7 @@ public void testSlackResponseObject() throws Exception { when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean())).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), any(Item.class))).thenReturn(slackServiceMock); String savedResponse = IOUtils.toString( this.getClass().getResourceAsStream("response.json") @@ -275,7 +281,7 @@ public void testSlackResponseObjectNullNonBotUser() throws Exception { when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean())).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), any(Item.class))).thenReturn(slackServiceMock); when(slackServiceMock.getResponseString()).thenReturn(null); when(slackServiceMock.publish(anyString(), anyString())).thenReturn(true); From 8d05af3626d868a40395a5399b035b124b6d9001 Mon Sep 17 00:00:00 2001 From: Marc Salles Date: Thu, 14 Mar 2019 17:22:22 +0100 Subject: [PATCH 02/10] Adapt code so pipelines work too --- .../jenkins/plugins/slack/SlackNotifier.java | 4 +- .../plugins/slack/workflow/SlackSendStep.java | 49 ++++++++++++---- .../plugins/slack/SlackNotifierStub.java | 2 +- .../slack/workflow/SlackSendStepTest.java | 57 +++++++++++++++---- 4 files changed, 89 insertions(+), 23 deletions(-) diff --git a/src/main/java/jenkins/plugins/slack/SlackNotifier.java b/src/main/java/jenkins/plugins/slack/SlackNotifier.java index ae65490a..1f6aff28 100755 --- a/src/main/java/jenkins/plugins/slack/SlackNotifier.java +++ b/src/main/java/jenkins/plugins/slack/SlackNotifier.java @@ -604,8 +604,8 @@ public boolean configure(StaplerRequest req, JSONObject formData) { return true; } - SlackService getSlackService(final String baseUrl, final String teamDomain, final String authTokenCredentialId, final boolean botUser, final String room, final Item project) { - return new StandardSlackService(baseUrl, teamDomain, authTokenCredentialId, botUser, room, project); + SlackService getSlackService(final String baseUrl, final String teamDomain, final String authTokenCredentialId, final boolean botUser, final String room, final Item item) { + return new StandardSlackService(baseUrl, teamDomain, authTokenCredentialId, botUser, room, item); } @Nonnull diff --git a/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java b/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java index 85124263..9d754bb9 100644 --- a/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java +++ b/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java @@ -34,9 +34,11 @@ import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.DataBoundSetter; import org.kohsuke.stapler.QueryParameter; +import java.util.logging.Level; import java.util.Objects; import java.util.Set; +import java.util.logging.Logger; import javax.annotation.Nonnull; import static com.cloudbees.plugins.credentials.CredentialsProvider.lookupCredentials; @@ -46,6 +48,8 @@ */ public class SlackSendStep extends Step { + private static final Logger logger = Logger.getLogger(SlackSendStep.class.getName()); + private String message; private String color; private String token; @@ -232,7 +236,7 @@ public static class SlackSendStepExecution extends SynchronousNonBlockingStepExe protected SlackResponse run() throws Exception { Jenkins jenkins = Jenkins.get(); - Project project = getContext().get(Project.class); + Item item = getItemForCredentials(); SlackNotifier.DescriptorImpl slackDesc = jenkins.getDescriptorByType(SlackNotifier.DescriptorImpl.class); String baseUrl = step.baseUrl != null ? step.baseUrl : slackDesc.getBaseUrl(); @@ -253,7 +257,7 @@ protected SlackResponse run() throws Exception { ); SlackService slackService = getSlackService( - baseUrl, teamDomain, token, tokenCredentialId, botUser, channel, step.replyBroadcast, project + baseUrl, teamDomain, token, tokenCredentialId, botUser, channel, step.replyBroadcast, item ); final boolean publishSuccess; if (step.attachments != null) { @@ -300,13 +304,12 @@ protected SlackResponse run() throws Exception { JSONArray getAttachmentsAsJSONArray() throws Exception { final TaskListener listener = getContext().get(TaskListener.class); - final String jsonString; - if (step.attachments instanceof String) { - jsonString = (String) step.attachments; - } - else { - jsonString = JsonOutput.toJson(step.attachments); - } + final String jsonString; + if (step.attachments instanceof String) { + jsonString = (String) step.attachments; + } else { + jsonString = JsonOutput.toJson(step.attachments); + } JsonSlurper jsonSlurper = new JsonSlurper(); JSON json = null; @@ -316,13 +319,39 @@ JSONArray getAttachmentsAsJSONArray() throws Exception { listener.error(Messages.NotificationFailedWithException(e)); return null; } - if(!(json instanceof JSONArray)){ + if (!(json instanceof JSONArray)) { listener.error(Messages.NotificationFailedWithException(new IllegalArgumentException("Attachments must be JSONArray"))); return null; } return (JSONArray) json; } + /** + * Tries to obtain the proper Item object to provide to CredentialsProvider. + * Project works for freestyle jobs, the parent of the Run works for pipelines. + * In case the proper item cannot be found, null is returned, since when null is provided to CredentialsProvider, + * it will internally use Jenkins.getInstance() which effectively only allows global credentials. + * + * @return the item to use for CredentialsProvider credential lookup + */ + private Item getItemForCredentials() { + Item item = null; + try { + item = getContext().get(Project.class); + if (item == null) { + Run run = getContext().get(Run.class); + if (run != null) { + item = run.getParent(); + } else { + item = null; + } + } + } catch (Exception e) { + logger.log(Level.INFO, "Exception obtaining item for credentials lookup. Only global credentials will be available", e); + } + return item; + } + private String defaultIfEmpty(String value) { return Util.fixEmpty(value) != null ? value : Messages.SlackSendStepValuesEmptyMessage(); } diff --git a/src/test/java/jenkins/plugins/slack/SlackNotifierStub.java b/src/test/java/jenkins/plugins/slack/SlackNotifierStub.java index ef2322a4..aafb57cf 100644 --- a/src/test/java/jenkins/plugins/slack/SlackNotifierStub.java +++ b/src/test/java/jenkins/plugins/slack/SlackNotifierStub.java @@ -25,7 +25,7 @@ public synchronized void load() { } @Override - SlackService getSlackService(final String baseUrl, final String teamDomain, final String authTokenCredentialId, final boolean botUser, final String room, final Item project) { + SlackService getSlackService(final String baseUrl, final String teamDomain, final String authTokenCredentialId, final boolean botUser, final String room, final Item item) { return slackService; } diff --git a/src/test/java/jenkins/plugins/slack/workflow/SlackSendStepTest.java b/src/test/java/jenkins/plugins/slack/workflow/SlackSendStepTest.java index c54c8d5a..f06566f9 100644 --- a/src/test/java/jenkins/plugins/slack/workflow/SlackSendStepTest.java +++ b/src/test/java/jenkins/plugins/slack/workflow/SlackSendStepTest.java @@ -2,6 +2,7 @@ import hudson.model.Item; import hudson.model.Project; +import hudson.model.Run; import hudson.model.TaskListener; import jenkins.model.Jenkins; import jenkins.plugins.slack.SlackNotifier; @@ -27,7 +28,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; -import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; +import static org.mockito.Matchers.isNull; import static org.mockito.Mockito.anyBoolean; import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.doNothing; @@ -52,6 +54,8 @@ public class SlackSendStepTest { @Mock Project project; @Mock + Run run; + @Mock SlackService slackServiceMock; @Mock Jenkins jenkins; @@ -64,7 +68,6 @@ public void setUp() throws IOException, InterruptedException { when(jenkins.getDescriptorByType(SlackNotifier.DescriptorImpl.class)).thenReturn(slackDescMock); when(taskListenerMock.getLogger()).thenReturn(printStreamMock); when(stepContextMock.get(TaskListener.class)).thenReturn(taskListenerMock); - when(stepContextMock.get(Project.class)).thenReturn(project); } @Test @@ -83,12 +86,14 @@ public void testStepOverrides() throws Exception { when(Jenkins.get()).thenReturn(jenkins); + when(stepContextMock.get(Project.class)).thenReturn(project); + when(slackDescMock.isBotUser()).thenReturn(false); when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), any(Item.class))).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), eq(project))).thenReturn(slackServiceMock); when(slackServiceMock.publish(anyString(), anyString())).thenReturn(true); stepExecution.run(); @@ -115,7 +120,7 @@ public void testStepWithAttachments() throws Exception { when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), any(Item.class))).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), isNull(Item.class) )).thenReturn(slackServiceMock); stepExecution.run(); verify(slackServiceMock, times(0)).publish("message", ""); @@ -141,7 +146,7 @@ public void testStepWithAttachmentsAsListOfMap() throws Exception { when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean())).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), isNull(Item.class))).thenReturn(slackServiceMock); stepExecution.run(); verify(slackServiceMock, times(0)).publish("message", ""); @@ -164,6 +169,36 @@ public void testValuesForGlobalConfig() throws Exception { SlackSendStep.SlackSendStepExecution stepExecution = spy(new SlackSendStep.SlackSendStepExecution(step, stepContextMock)); when(Jenkins.get()).thenReturn(jenkins); + when(stepContextMock.get(Project.class)).thenReturn(project); + + when(slackDescMock.getBaseUrl()).thenReturn("globalBaseUrl"); + when(slackDescMock.getTeamDomain()).thenReturn("globalTeamDomain"); + when(slackDescMock.getTokenCredentialId()).thenReturn("globalTokenCredentialId"); + when(slackDescMock.isBotUser()).thenReturn(false); + when(slackDescMock.getRoom()).thenReturn("globalChannel"); + + when(taskListenerMock.getLogger()).thenReturn(printStreamMock); + doNothing().when(printStreamMock).println(); + + when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), eq(project))).thenReturn(slackServiceMock); + + stepExecution.run(); + verify(stepExecution, times(1)).getSlackService("globalBaseUrl", "globalTeamDomain", null, "globalTokenCredentialId", false, "globalChannel", false, project); + verify(slackServiceMock, times(1)).publish("message", ""); + } + + + @Test + public void testCanGetItemFromRun() throws Exception { + SlackSendStep step = new SlackSendStep(); + step.setMessage("message"); + + SlackSendStep.SlackSendStepExecution stepExecution = spy(new SlackSendStep.SlackSendStepExecution(step, stepContextMock)); + when(Jenkins.get()).thenReturn(jenkins); + + when(stepContextMock.get(Run.class)).thenReturn(run); + when(run.getParent()).thenReturn(project); + when(slackDescMock.getBaseUrl()).thenReturn("globalBaseUrl"); when(slackDescMock.getTeamDomain()).thenReturn("globalTeamDomain"); when(slackDescMock.getTokenCredentialId()).thenReturn("globalTokenCredentialId"); @@ -173,7 +208,7 @@ public void testValuesForGlobalConfig() throws Exception { when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), any(Item.class))).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), eq(project))).thenReturn(slackServiceMock); stepExecution.run(); verify(stepExecution, times(1)).getSlackService("globalBaseUrl", "globalTeamDomain", null, "globalTokenCredentialId", false, "globalChannel", false, project); @@ -190,6 +225,8 @@ public void testReplyBroadcast() throws Exception { when(Jenkins.get()).thenReturn(jenkins); + when(stepContextMock.get(Project.class)).thenReturn(project); + when(slackDescMock.getBaseUrl()).thenReturn("globalBaseUrl"); when(slackDescMock.getTeamDomain()).thenReturn("globalTeamDomain"); when(slackDescMock.getTokenCredentialId()).thenReturn("globalTokenCredentialId"); @@ -199,7 +236,7 @@ public void testReplyBroadcast() throws Exception { when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), any(Item.class))).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), eq(project))).thenReturn(slackServiceMock); stepExecution.run(); verify(stepExecution, times(1)).getSlackService("globalBaseUrl", "globalTeamDomain", null, "globalTokenCredentialId", false, "globalChannel", true, project); @@ -218,7 +255,7 @@ public void testNonNullEmptyColor() throws Exception { when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), any(Item.class))).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), isNull(Item.class))).thenReturn(slackServiceMock); stepExecution.run(); verify(slackServiceMock, times(1)).publish("message", ""); @@ -243,7 +280,7 @@ public void testSlackResponseObject() throws Exception { when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), any(Item.class))).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), isNull(Item.class))).thenReturn(slackServiceMock); String savedResponse = IOUtils.toString( this.getClass().getResourceAsStream("response.json") @@ -281,7 +318,7 @@ public void testSlackResponseObjectNullNonBotUser() throws Exception { when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), any(Item.class))).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), isNull(Item.class))).thenReturn(slackServiceMock); when(slackServiceMock.getResponseString()).thenReturn(null); when(slackServiceMock.publish(anyString(), anyString())).thenReturn(true); From 1c8d88ec95bf6cfc4288e69e17a3fe7ae58c1c7e Mon Sep 17 00:00:00 2001 From: Marc Salles Date: Fri, 15 Mar 2019 13:13:10 +0100 Subject: [PATCH 03/10] undoing modification of previous constructors for StandardSlackService --- .../java/jenkins/plugins/slack/SlackNotifier.java | 7 ++++--- .../jenkins/plugins/slack/StandardSlackService.java | 12 ++++++++---- .../plugins/slack/StandardSlackServiceStub.java | 2 +- .../plugins/slack/StandardSlackServiceTest.java | 6 +++--- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/main/java/jenkins/plugins/slack/SlackNotifier.java b/src/main/java/jenkins/plugins/slack/SlackNotifier.java index 1f6aff28..83ba901e 100755 --- a/src/main/java/jenkins/plugins/slack/SlackNotifier.java +++ b/src/main/java/jenkins/plugins/slack/SlackNotifier.java @@ -428,7 +428,8 @@ public SlackService newSlackService(AbstractBuild r, BuildListener listener) { authToken = env.expand(authToken); authTokenCredentialId = env.expand(authTokenCredentialId); room = env.expand(room); - return new StandardSlackService(baseUrl, teamDomain, authToken, authTokenCredentialId, botUser, room, r.getProject()); + return new StandardSlackService(baseUrl, teamDomain, authToken, authTokenCredentialId, botUser, room, false, r.getProject()); + } @Override @@ -604,8 +605,8 @@ public boolean configure(StaplerRequest req, JSONObject formData) { return true; } - SlackService getSlackService(final String baseUrl, final String teamDomain, final String authTokenCredentialId, final boolean botUser, final String room, final Item item) { - return new StandardSlackService(baseUrl, teamDomain, authTokenCredentialId, botUser, room, item); + SlackService getSlackService(final String baseUrl, final String teamDomain, final String authTokenCredentialId, final boolean botUser, final String roomId, final Item item) { + return new StandardSlackService(baseUrl, teamDomain, null, authTokenCredentialId, botUser, roomId, false, item); } @Nonnull diff --git a/src/main/java/jenkins/plugins/slack/StandardSlackService.java b/src/main/java/jenkins/plugins/slack/StandardSlackService.java index e668bab9..99ca1219 100755 --- a/src/main/java/jenkins/plugins/slack/StandardSlackService.java +++ b/src/main/java/jenkins/plugins/slack/StandardSlackService.java @@ -53,12 +53,16 @@ public class StandardSlackService implements SlackService { private String responseString = null; private Item item; - public StandardSlackService(String baseUrl, String teamDomain, String authTokenCredentialId, boolean botUser, String roomId, Item item) { - this(baseUrl, teamDomain, null, authTokenCredentialId, botUser, roomId, false, item); + public StandardSlackService(String baseUrl, String teamDomain, String authTokenCredentialId, boolean botUser, String roomId) { + this(baseUrl, teamDomain, null, authTokenCredentialId, botUser, roomId, false, null); } - public StandardSlackService(String baseUrl, String teamDomain, String token, String authTokenCredentialId, boolean botUser, String roomId, Item item) { - this(baseUrl, teamDomain, token, authTokenCredentialId, botUser, roomId, false, item); + public StandardSlackService(String baseUrl, String teamDomain, String token, String authTokenCredentialId, boolean botUser, String roomId) { + this(baseUrl, teamDomain, token, authTokenCredentialId, botUser, roomId, false, null); + } + + public StandardSlackService(String baseUrl, String teamDomain, String token, String authTokenCredentialId, boolean botUser, String roomId, boolean replyBroadcast) { + this(baseUrl, teamDomain, token, authTokenCredentialId, botUser, roomId, replyBroadcast, null); } public StandardSlackService(String baseUrl, String teamDomain, String token, String authTokenCredentialId, boolean botUser, String roomId, boolean replyBroadcast, Item item) { diff --git a/src/test/java/jenkins/plugins/slack/StandardSlackServiceStub.java b/src/test/java/jenkins/plugins/slack/StandardSlackServiceStub.java index 89f6c160..f6aa65e4 100644 --- a/src/test/java/jenkins/plugins/slack/StandardSlackServiceStub.java +++ b/src/test/java/jenkins/plugins/slack/StandardSlackServiceStub.java @@ -7,7 +7,7 @@ public class StandardSlackServiceStub extends StandardSlackService { private CloseableHttpClientStub httpClientStub; public StandardSlackServiceStub(String baseUrl, String teamDomain, String token, String tokenCredentialId, boolean botUser, String roomId, Item item) { - super(baseUrl, teamDomain, token, tokenCredentialId, botUser, roomId, item); + super(baseUrl, teamDomain, token, tokenCredentialId, botUser, roomId, false, item); } @Override diff --git a/src/test/java/jenkins/plugins/slack/StandardSlackServiceTest.java b/src/test/java/jenkins/plugins/slack/StandardSlackServiceTest.java index b3cb6506..5e43538c 100755 --- a/src/test/java/jenkins/plugins/slack/StandardSlackServiceTest.java +++ b/src/test/java/jenkins/plugins/slack/StandardSlackServiceTest.java @@ -15,7 +15,7 @@ public class StandardSlackServiceTest { */ @Test public void publishWithBadHostShouldNotRethrowExceptions() { - StandardSlackService service = new StandardSlackService("", "foo", "token", null, false, "#general", null); + StandardSlackService service = new StandardSlackService("", "foo", "token", null, false, "#general"); service.setHost("hostvaluethatwillcausepublishtofail"); service.publish("message"); } @@ -25,7 +25,7 @@ public void publishWithBadHostShouldNotRethrowExceptions() { */ @Test public void invalidTeamDomainShouldFail() { - StandardSlackService service = new StandardSlackService("", "my", "token", null, false, "#general", null); + StandardSlackService service = new StandardSlackService("", "my", "token", null, false, "#general"); service.publish("message"); } @@ -34,7 +34,7 @@ public void invalidTeamDomainShouldFail() { */ @Test public void invalidTokenShouldFail() { - StandardSlackService service = new StandardSlackService("", "tinyspeck", "token", null, false, "#general", null); + StandardSlackService service = new StandardSlackService("", "tinyspeck", "token", null, false, "#general"); service.publish("message"); } From 7d3c7e98b6e651e218577e5715c0ae53410900e3 Mon Sep 17 00:00:00 2001 From: Marc Salles Date: Fri, 15 Mar 2019 17:09:17 +0100 Subject: [PATCH 04/10] modified service constructor so it takes a String instead of an Item --- .../plugins/slack/CredentialsObtainer.java | 41 ++++++++++ .../jenkins/plugins/slack/SlackNotifier.java | 18 ++++- .../plugins/slack/StandardSlackService.java | 74 +++++++++++++------ .../plugins/slack/workflow/SlackSendStep.java | 16 ++-- .../StandardSlackServiceCredentialsTest.java | 54 +++----------- .../slack/StandardSlackServiceStub.java | 10 ++- .../slack/StandardSlackServiceTest.java | 25 +++---- .../slack/workflow/SlackSendStepTest.java | 60 ++++++++++----- 8 files changed, 186 insertions(+), 112 deletions(-) create mode 100644 src/main/java/jenkins/plugins/slack/CredentialsObtainer.java diff --git a/src/main/java/jenkins/plugins/slack/CredentialsObtainer.java b/src/main/java/jenkins/plugins/slack/CredentialsObtainer.java new file mode 100644 index 00000000..88d23f37 --- /dev/null +++ b/src/main/java/jenkins/plugins/slack/CredentialsObtainer.java @@ -0,0 +1,41 @@ +package jenkins.plugins.slack; + +import com.cloudbees.plugins.credentials.CredentialsMatcher; +import com.cloudbees.plugins.credentials.CredentialsMatchers; +import hudson.model.Item; +import hudson.security.ACL; +import org.apache.commons.lang.StringUtils; +import org.jenkinsci.plugins.plaincredentials.StringCredentials; + +import java.util.Collections; +import java.util.List; + +public class CredentialsObtainer { + + public static StringCredentials lookupCredentials(String credentialId, Item item) { + List credentials = com.cloudbees.plugins.credentials.CredentialsProvider.lookupCredentials(StringCredentials.class, item, ACL.SYSTEM, Collections.emptyList()); + CredentialsMatcher matcher = CredentialsMatchers.withId(credentialId); + return CredentialsMatchers.firstOrNull(credentials, matcher); + } + + /** + * Attempts to obtain the credential with the providedId from the item's credential context, otherwise returns token + * @param credentialId the id from the credential to be used + * @param item the item with the context to obtain the credential from. + * @param token the fallback token + * @return the obtained token + */ + public static String getTokenToUse(String credentialId, Item item, String token) { + if (StringUtils.isEmpty(credentialId)) { + return token; + } + StringCredentials credentials = lookupCredentials(StringUtils.trim(credentialId), item); + final String response; + if (credentials != null) { + response = credentials.getSecret().getPlainText(); + } else { + response = token; + } + return response; + } +} diff --git a/src/main/java/jenkins/plugins/slack/SlackNotifier.java b/src/main/java/jenkins/plugins/slack/SlackNotifier.java index 83ba901e..af858d5e 100755 --- a/src/main/java/jenkins/plugins/slack/SlackNotifier.java +++ b/src/main/java/jenkins/plugins/slack/SlackNotifier.java @@ -1,5 +1,6 @@ package jenkins.plugins.slack; +import com.cloudbees.plugins.credentials.Credentials; import com.cloudbees.plugins.credentials.common.StandardListBoxModel; import com.cloudbees.plugins.credentials.domains.HostnameRequirement; import hudson.EnvVars; @@ -34,6 +35,7 @@ import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.StaplerRequest; +import java.util.NoSuchElementException; import java.util.function.Function; import java.util.logging.Level; import java.util.logging.Logger; @@ -428,7 +430,14 @@ public SlackService newSlackService(AbstractBuild r, BuildListener listener) { authToken = env.expand(authToken); authTokenCredentialId = env.expand(authTokenCredentialId); room = env.expand(room); - return new StandardSlackService(baseUrl, teamDomain, authToken, authTokenCredentialId, botUser, room, false, r.getProject()); + final String populatedToken = CredentialsObtainer.getTokenToUse(authTokenCredentialId, r.getProject(), authToken); + if (populatedToken != null) { + return new StandardSlackService(baseUrl, teamDomain, botUser, room, false, populatedToken); + } else { + logger.log(Level.INFO, "No explicit token specified and could not obtain credentials for id: " + authTokenCredentialId); + return new StandardSlackService(baseUrl, teamDomain, authToken, authTokenCredentialId, botUser, room, false); + } + } @@ -606,7 +615,12 @@ public boolean configure(StaplerRequest req, JSONObject formData) { } SlackService getSlackService(final String baseUrl, final String teamDomain, final String authTokenCredentialId, final boolean botUser, final String roomId, final Item item) { - return new StandardSlackService(baseUrl, teamDomain, null, authTokenCredentialId, botUser, roomId, false, item); + final String populatedToken = CredentialsObtainer.getTokenToUse(authTokenCredentialId, item,null ); + if (populatedToken != null) { + return new StandardSlackService(baseUrl, teamDomain, botUser, roomId, false, populatedToken); + } else { + throw new NoSuchElementException("Could not obtain credentials with credential id: " + authTokenCredentialId); + } } @Nonnull diff --git a/src/main/java/jenkins/plugins/slack/StandardSlackService.java b/src/main/java/jenkins/plugins/slack/StandardSlackService.java index 99ca1219..7d241aba 100755 --- a/src/main/java/jenkins/plugins/slack/StandardSlackService.java +++ b/src/main/java/jenkins/plugins/slack/StandardSlackService.java @@ -45,41 +45,70 @@ public class StandardSlackService implements SlackService { private String host = "slack.com"; private String baseUrl; private String teamDomain; - private String token; - private String authTokenCredentialId; + private String token = null; + private String authTokenCredentialId = null; private boolean botUser; private String[] roomIds; private boolean replyBroadcast; private String responseString = null; - private Item item; + private String populatedToken = null; + /** + * @deprecated use {@link #StandardSlackService(String, String, boolean, String, boolean, String)} instead} + */ + @Deprecated public StandardSlackService(String baseUrl, String teamDomain, String authTokenCredentialId, boolean botUser, String roomId) { - this(baseUrl, teamDomain, null, authTokenCredentialId, botUser, roomId, false, null); + this(baseUrl, teamDomain, null, authTokenCredentialId, botUser, roomId, false); } + /** + * @deprecated use {@link #StandardSlackService(String, String, boolean, String, boolean, String)} instead} + */ + @Deprecated public StandardSlackService(String baseUrl, String teamDomain, String token, String authTokenCredentialId, boolean botUser, String roomId) { - this(baseUrl, teamDomain, token, authTokenCredentialId, botUser, roomId, false, null); + this(baseUrl, teamDomain, token, authTokenCredentialId, botUser, roomId, false); } + /** + * @deprecated use {@link #StandardSlackService(String, String, boolean, String, boolean, String)} instead} + */ + @Deprecated public StandardSlackService(String baseUrl, String teamDomain, String token, String authTokenCredentialId, boolean botUser, String roomId, boolean replyBroadcast) { - this(baseUrl, teamDomain, token, authTokenCredentialId, botUser, roomId, replyBroadcast, null); + this(baseUrl, teamDomain, botUser, roomId, replyBroadcast); + this.token = token; + this.authTokenCredentialId = StringUtils.trim(authTokenCredentialId); + } + + /** + * @param baseUrl the full url to use, this is an alternative to specifying teamDomain + * @param teamDomain the teamDomain inside slack.com to use + * @param botUser + * @param roomId a semicolon separated list of rooms to notify + * @param replyBroadcast + * @param populatedToken a non-null token to use for authentication + */ + public StandardSlackService(String baseUrl, String teamDomain, boolean botUser, String roomId, boolean replyBroadcast, String populatedToken) { + this(baseUrl, teamDomain, botUser, roomId, replyBroadcast); + if (populatedToken == null) { + throw new IllegalArgumentException("populatedToken cannot be null"); + } + this.populatedToken = populatedToken; } - public StandardSlackService(String baseUrl, String teamDomain, String token, String authTokenCredentialId, boolean botUser, String roomId, boolean replyBroadcast, Item item) { + private StandardSlackService(String baseUrl, String teamDomain, boolean botUser, String roomId, boolean replyBroadcast) { super(); this.baseUrl = baseUrl; if(this.baseUrl != null && !this.baseUrl.isEmpty() && !this.baseUrl.endsWith("/")) { this.baseUrl += "/"; } this.teamDomain = teamDomain; - this.token = token; - this.authTokenCredentialId = StringUtils.trim(authTokenCredentialId); this.botUser = botUser; this.roomIds = roomId.split("[,; ]+"); this.replyBroadcast = replyBroadcast; - this.item = item; } + + public String getResponseString() { return responseString; } @@ -193,25 +222,22 @@ public boolean publish(String message, JSONArray attachments, String color) { } private String getTokenToUse() { - if (authTokenCredentialId != null && !authTokenCredentialId.isEmpty()) { - StringCredentials credentials = lookupCredentials(authTokenCredentialId); - if (credentials != null) { - logger.fine("Using Integration Token Credential ID."); - return credentials.getSecret().getPlainText(); + if (populatedToken != null) { + return populatedToken; + } else { + if (!StringUtils.isEmpty(authTokenCredentialId)) { + StringCredentials credentials = CredentialsObtainer.lookupCredentials(authTokenCredentialId, null); + if (credentials != null) { + logger.fine("Using Integration Token Credential ID."); + return credentials.getSecret().getPlainText(); + } } - } - - logger.fine("Using Integration Token."); + logger.fine("Using Integration Token."); + } return token; } - private StringCredentials lookupCredentials(String credentialId) { - List credentials = com.cloudbees.plugins.credentials.CredentialsProvider.lookupCredentials(StringCredentials.class, item, ACL.SYSTEM, Collections.emptyList()); - CredentialsMatcher matcher = CredentialsMatchers.withId(credentialId); - return CredentialsMatchers.firstOrNull(credentials, matcher); - } - protected CloseableHttpClient getHttpClient() { final HttpClientBuilder clientBuilder = HttpClients.custom(); final CredentialsProvider credentialsProvider = new BasicCredentialsProvider(); diff --git a/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java b/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java index 9d754bb9..db522c27 100644 --- a/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java +++ b/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java @@ -15,6 +15,7 @@ import hudson.util.FormValidation; import hudson.util.ListBoxModel; import jenkins.model.Jenkins; +import jenkins.plugins.slack.CredentialsObtainer; import jenkins.plugins.slack.Messages; import jenkins.plugins.slack.SlackNotifier; import jenkins.plugins.slack.SlackService; @@ -63,6 +64,7 @@ public class SlackSendStep extends Step { private boolean replyBroadcast; + @Nonnull public String getMessage() { return message; @@ -255,10 +257,14 @@ protected SlackResponse run() throws Exception { defaultIfEmpty(baseUrl), defaultIfEmpty(teamDomain), channel, defaultIfEmpty(color), botUser, defaultIfEmpty(tokenCredentialId)) ); - + final String populatedToken = CredentialsObtainer.getTokenToUse(tokenCredentialId, item, token); + if (populatedToken == null) { + listener.error(Messages + .NotificationFailedWithException(new IllegalArgumentException("the token with the provided ID could not be found and no token was specified"))); + return null; + } SlackService slackService = getSlackService( - baseUrl, teamDomain, token, tokenCredentialId, botUser, channel, step.replyBroadcast, item - ); + baseUrl, teamDomain, botUser, channel, step.replyBroadcast, populatedToken); final boolean publishSuccess; if (step.attachments != null) { JSONArray jsonArray = getAttachmentsAsJSONArray(); @@ -357,8 +363,8 @@ private String defaultIfEmpty(String value) { } //streamline unit testing - SlackService getSlackService(String baseUrl, String team, String token, String tokenCredentialId, boolean botUser, String channel, boolean replyBroadcast, Item item) { - return new StandardSlackService(baseUrl, team, token, tokenCredentialId, botUser, channel, replyBroadcast, item); + SlackService getSlackService(String baseUrl, String team, boolean botUser, String channel, boolean replyBroadcast, String populatedToken) { + return new StandardSlackService(baseUrl, team, botUser, channel, replyBroadcast, populatedToken); } } } diff --git a/src/test/java/jenkins/plugins/slack/StandardSlackServiceCredentialsTest.java b/src/test/java/jenkins/plugins/slack/StandardSlackServiceCredentialsTest.java index eb241d3e..a1b8e698 100644 --- a/src/test/java/jenkins/plugins/slack/StandardSlackServiceCredentialsTest.java +++ b/src/test/java/jenkins/plugins/slack/StandardSlackServiceCredentialsTest.java @@ -2,7 +2,6 @@ import com.cloudbees.plugins.credentials.CredentialsProvider; import hudson.ExtensionList; -import hudson.model.*; import hudson.security.ACL; import hudson.util.Secret; import jenkins.model.Jenkins; @@ -21,7 +20,6 @@ import java.util.Collections; import java.util.List; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.powermock.api.mockito.PowerMockito.mock; import static org.powermock.api.mockito.PowerMockito.when; @@ -38,24 +36,22 @@ public void setUp() { } @Test - public void providedJobCredentialsAreUsed() { - final String id = "cred-id"; - final String secretText = "secret-text"; - final Job job = createJobWithAvailableCredentialId(id, secretText); - final StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", null, id, true, "#room1:1528317530", job); + public void populatedTokenIsUsed() { + final String populatedToken = "secret-text"; + final StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", true, "#room1:1528317530", populatedToken); final CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); service.publish("message"); - assertTrue(httpClientStub.getLastRequest().getURI().toString().contains(secretText)); + assertTrue(httpClientStub.getLastRequest().getURI().toString().contains(populatedToken)); } @Test - public void globalCredentialsCanBeUsed() { + public void globalCredentialByIdUsed() { final String id = "cred-2id"; final String secretText = "secret-2text"; setupGlobalAvailableCredentialId(id, secretText); - final StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", null, id, true, "#room1:1528317530", null); + final StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", null, id, true, "#room1:1528317530"); final CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); @@ -64,22 +60,9 @@ public void globalCredentialsCanBeUsed() { } @Test - public void otherJobCredentialsAreNotObtained() { - final String id = "cred-id"; - final String secretText = "secret-text"; - final Job job = createJobWithAvailableCredentialId(id, secretText); - final StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", null, "wrongid", true, "#room1:1528317530", job); - final CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); - httpClientStub.setHttpStatus(HttpStatus.SC_OK); - service.setHttpClient(httpClientStub); - service.publish("message"); - assertFalse(httpClientStub.getLastRequest().getURI().toString().contains(secretText)); - } - - @Test - public void ifNoJobProvidedTokenIsUsed() { + public void tokenIsUsed() { final String token = "explicittoken"; - final StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", token, null, true, "#room1:1528317530", null); + final StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", token, null, true, "#room1:1528317530"); final CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); @@ -87,27 +70,8 @@ public void ifNoJobProvidedTokenIsUsed() { assertTrue(httpClientStub.getLastRequest().getURI().toString().contains(token)); } - /** - * Creates a mocked job and sets up the mocking mechanism so the provided credential is obtainable. - * @param id the id of the credential to use. - * @param secretText the secret text of the credential. - * @return a mocked job set up so it can use the provided credentials - */ - private Job createJobWithAvailableCredentialId(final String id, final String secretText) { - final Job job = mock(Job.class); - final CredentialsProvider credentialsProvider = mock(CredentialsProvider.class); - final List credentialList = setupMocks(id, secretText, credentialsProvider); - when(credentialsProvider.getCredentials(Matchers.eq(StringCredentials.class), Matchers.eq(job), Matchers.eq(ACL.SYSTEM), Matchers.eq(Collections.emptyList()))).thenReturn(credentialList); - return job; - } - private void setupGlobalAvailableCredentialId(final String id, final String secretText) { final CredentialsProvider credentialsProvider = mock(CredentialsProvider.class); - final List credentialList = setupMocks(id, secretText, credentialsProvider); - when(credentialsProvider.getCredentials(Matchers.eq(StringCredentials.class), Matchers.eq(jenkins), Matchers.eq(ACL.SYSTEM), Matchers.eq(Collections.emptyList()))).thenReturn(credentialList); - } - - private List setupMocks(String id, String secretText, CredentialsProvider credentialsProvider) { final ExtensionList extensionList = mock(ExtensionList.class); final List listProviders = new ArrayList<>(1); final List credentialList = new ArrayList<>(); @@ -121,7 +85,7 @@ private List setupMocks(String id, String secretText, Credent when(secret.getPlainText()).thenReturn(secretText); when(credentials.getSecret()).thenReturn(secret); credentialList.add(credentials); - return credentialList; + when(credentialsProvider.getCredentials(Matchers.eq(StringCredentials.class), Matchers.eq(jenkins), Matchers.eq(ACL.SYSTEM), Matchers.eq(Collections.emptyList()))).thenReturn(credentialList); } } diff --git a/src/test/java/jenkins/plugins/slack/StandardSlackServiceStub.java b/src/test/java/jenkins/plugins/slack/StandardSlackServiceStub.java index f6aa65e4..1d2a3bc5 100644 --- a/src/test/java/jenkins/plugins/slack/StandardSlackServiceStub.java +++ b/src/test/java/jenkins/plugins/slack/StandardSlackServiceStub.java @@ -1,13 +1,15 @@ package jenkins.plugins.slack; -import hudson.model.Item; - public class StandardSlackServiceStub extends StandardSlackService { private CloseableHttpClientStub httpClientStub; - public StandardSlackServiceStub(String baseUrl, String teamDomain, String token, String tokenCredentialId, boolean botUser, String roomId, Item item) { - super(baseUrl, teamDomain, token, tokenCredentialId, botUser, roomId, false, item); + public StandardSlackServiceStub(String baseUrl, String teamDomain, String token, String tokenCredentialId, boolean botUser, String roomId) { + super(baseUrl, teamDomain, token, tokenCredentialId, botUser, roomId, false); + } + + public StandardSlackServiceStub(String baseUrl, String teamDomain, boolean botUser, String roomId, String populatedToken) { + super(baseUrl, teamDomain, botUser, roomId, false, populatedToken); } @Override diff --git a/src/test/java/jenkins/plugins/slack/StandardSlackServiceTest.java b/src/test/java/jenkins/plugins/slack/StandardSlackServiceTest.java index 5e43538c..aeb8a897 100755 --- a/src/test/java/jenkins/plugins/slack/StandardSlackServiceTest.java +++ b/src/test/java/jenkins/plugins/slack/StandardSlackServiceTest.java @@ -1,13 +1,11 @@ package jenkins.plugins.slack; -import hudson.model.Job; import org.apache.http.HttpStatus; import org.junit.Test; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.mock; public class StandardSlackServiceTest { /** @@ -40,7 +38,7 @@ public void invalidTokenShouldFail() { @Test public void publishToASingleRoomSendsASingleMessage() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1", null); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1"); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); service.setHttpClient(httpClientStub); service.publish("message"); @@ -49,7 +47,7 @@ public void publishToASingleRoomSendsASingleMessage() { @Test public void publishToMultipleRoomsSendsAMessageToEveryRoom() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1,#room2,#room3", null); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1,#room2,#room3"); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); service.setHttpClient(httpClientStub); service.publish("message"); @@ -58,7 +56,7 @@ public void publishToMultipleRoomsSendsAMessageToEveryRoom() { @Test public void successfulPublishToASingleRoomReturnsTrue() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1", null); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1"); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); @@ -67,9 +65,8 @@ public void successfulPublishToASingleRoomReturnsTrue() { @Test - public void successfulPublishToSingleRoomWithProvidedJobReturnsTrue() { - Job job = mock(Job.class); - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1", job); + public void successfulPublishToSingleRoomWithProvidedTokenReturnsTrue() { + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", false, "#room1", "providedtoken"); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); @@ -78,7 +75,7 @@ public void successfulPublishToSingleRoomWithProvidedJobReturnsTrue() { @Test public void successfulPublishToMultipleRoomsReturnsTrue() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1,#room2,#room3", null); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1,#room2,#room3"); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); @@ -87,7 +84,7 @@ public void successfulPublishToMultipleRoomsReturnsTrue() { @Test public void failedPublishToASingleRoomReturnsFalse() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1", null); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1"); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_NOT_FOUND); service.setHttpClient(httpClientStub); @@ -96,7 +93,7 @@ public void failedPublishToASingleRoomReturnsFalse() { @Test public void singleFailedPublishToMultipleRoomsReturnsFalse() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1,#room2,#room3", null); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1,#room2,#room3"); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setFailAlternateResponses(true); httpClientStub.setHttpStatus(HttpStatus.SC_OK); @@ -106,7 +103,7 @@ public void singleFailedPublishToMultipleRoomsReturnsFalse() { @Test public void publishToEmptyRoomReturnsTrue() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "", null); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, ""); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); @@ -116,7 +113,7 @@ public void publishToEmptyRoomReturnsTrue() { @Test public void sendAsBotUserReturnsTrue() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, true, "#room1", null); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, true, "#room1"); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); @@ -125,7 +122,7 @@ public void sendAsBotUserReturnsTrue() { @Test public void sendAsBotUserInThreadReturnsTrue() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, true, "#room1:1528317530", null); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, true, "#room1:1528317530"); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); diff --git a/src/test/java/jenkins/plugins/slack/workflow/SlackSendStepTest.java b/src/test/java/jenkins/plugins/slack/workflow/SlackSendStepTest.java index f06566f9..08f91cb7 100644 --- a/src/test/java/jenkins/plugins/slack/workflow/SlackSendStepTest.java +++ b/src/test/java/jenkins/plugins/slack/workflow/SlackSendStepTest.java @@ -5,6 +5,7 @@ import hudson.model.Run; import hudson.model.TaskListener; import jenkins.model.Jenkins; +import jenkins.plugins.slack.CredentialsObtainer; import jenkins.plugins.slack.SlackNotifier; import jenkins.plugins.slack.SlackService; import net.sf.json.JSONArray; @@ -28,8 +29,9 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; + +import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; -import static org.mockito.Matchers.isNull; import static org.mockito.Mockito.anyBoolean; import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.doNothing; @@ -42,7 +44,7 @@ * Traditional Unit tests, allows testing null Jenkins.get() */ @RunWith(PowerMockRunner.class) -@PrepareForTest({Jenkins.class,SlackSendStep.class}) +@PrepareForTest({Jenkins.class,SlackSendStep.class, CredentialsObtainer.class}) public class SlackSendStepTest { @Mock @@ -65,27 +67,31 @@ public class SlackSendStepTest { @Before public void setUp() throws IOException, InterruptedException { PowerMockito.mockStatic(Jenkins.class); + PowerMockito.mockStatic(CredentialsObtainer.class); when(jenkins.getDescriptorByType(SlackNotifier.DescriptorImpl.class)).thenReturn(slackDescMock); + PowerMockito.when(Jenkins.getInstance()).thenReturn(jenkins); when(taskListenerMock.getLogger()).thenReturn(printStreamMock); when(stepContextMock.get(TaskListener.class)).thenReturn(taskListenerMock); } @Test public void testStepOverrides() throws Exception { + final String token = "mytoken"; SlackSendStep slackSendStep = new SlackSendStep(); slackSendStep.setMessage("message"); - slackSendStep.setToken("token"); + slackSendStep.setToken(token); slackSendStep.setTokenCredentialId("tokenCredentialId"); slackSendStep.setBotUser(true); slackSendStep.setBaseUrl("baseUrl/"); slackSendStep.setTeamDomain("teamDomain"); slackSendStep.setChannel("channel"); slackSendStep.setColor("good"); - SlackSendStep.SlackSendStepExecution stepExecution = spy(new SlackSendStep.SlackSendStepExecution(slackSendStep, stepContextMock)); when(Jenkins.get()).thenReturn(jenkins); + PowerMockito.when(CredentialsObtainer.getTokenToUse(anyString(), any(Item.class), anyString())).thenReturn(token); + when(stepContextMock.get(Project.class)).thenReturn(project); when(slackDescMock.isBotUser()).thenReturn(false); @@ -93,11 +99,11 @@ public void testStepOverrides() throws Exception { when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), eq(project))).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), anyString())).thenReturn(slackServiceMock); when(slackServiceMock.publish(anyString(), anyString())).thenReturn(true); stepExecution.run(); - verify(stepExecution, times(1)).getSlackService("baseUrl/", "teamDomain", "token", "tokenCredentialId", true, "channel", false, project); + verify(stepExecution, times(1)).getSlackService("baseUrl/", "teamDomain", true, "channel", false, token); verify(slackServiceMock, times(1)).publish("message", "good"); } @@ -117,10 +123,12 @@ public void testStepWithAttachments() throws Exception { when(Jenkins.get()).thenReturn(jenkins); + PowerMockito.when(CredentialsObtainer.getTokenToUse(anyString(), any(Item.class), anyString())).thenReturn("token"); + when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), isNull(Item.class) )).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), anyString() )).thenReturn(slackServiceMock); stepExecution.run(); verify(slackServiceMock, times(0)).publish("message", ""); @@ -143,10 +151,12 @@ public void testStepWithAttachmentsAsListOfMap() throws Exception { when(Jenkins.get()).thenReturn(jenkins); + PowerMockito.when(CredentialsObtainer.getTokenToUse(anyString(), any(Item.class), anyString())).thenReturn("token"); + when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), isNull(Item.class))).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), anyString())).thenReturn(slackServiceMock); stepExecution.run(); verify(slackServiceMock, times(0)).publish("message", ""); @@ -167,8 +177,11 @@ public void testValuesForGlobalConfig() throws Exception { step.setMessage("message"); SlackSendStep.SlackSendStepExecution stepExecution = spy(new SlackSendStep.SlackSendStepExecution(step, stepContextMock)); + when(Jenkins.get()).thenReturn(jenkins); + PowerMockito.when(CredentialsObtainer.getTokenToUse(eq("globalTokenCredentialId"), any(Item.class), anyString())).thenReturn("token2"); + when(stepContextMock.get(Project.class)).thenReturn(project); when(slackDescMock.getBaseUrl()).thenReturn("globalBaseUrl"); @@ -180,10 +193,10 @@ public void testValuesForGlobalConfig() throws Exception { when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), eq(project))).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), anyString())).thenReturn(slackServiceMock); stepExecution.run(); - verify(stepExecution, times(1)).getSlackService("globalBaseUrl", "globalTeamDomain", null, "globalTokenCredentialId", false, "globalChannel", false, project); + verify(stepExecution, times(1)).getSlackService("globalBaseUrl", "globalTeamDomain",false, "globalChannel", false, "token2"); verify(slackServiceMock, times(1)).publish("message", ""); } @@ -195,9 +208,9 @@ public void testCanGetItemFromRun() throws Exception { SlackSendStep.SlackSendStepExecution stepExecution = spy(new SlackSendStep.SlackSendStepExecution(step, stepContextMock)); when(Jenkins.get()).thenReturn(jenkins); - when(stepContextMock.get(Run.class)).thenReturn(run); when(run.getParent()).thenReturn(project); + PowerMockito.when(CredentialsObtainer.getTokenToUse(anyString(), eq(project), anyString())).thenReturn("runcredentials"); when(slackDescMock.getBaseUrl()).thenReturn("globalBaseUrl"); when(slackDescMock.getTeamDomain()).thenReturn("globalTeamDomain"); @@ -208,10 +221,12 @@ public void testCanGetItemFromRun() throws Exception { when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), eq(project))).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), anyString())).thenReturn(slackServiceMock); stepExecution.run(); - verify(stepExecution, times(1)).getSlackService("globalBaseUrl", "globalTeamDomain", null, "globalTokenCredentialId", false, "globalChannel", false, project); + + verify(stepExecution, times(1)).getSlackService("globalBaseUrl", "globalTeamDomain", + false, "globalChannel", false, "runcredentials"); verify(slackServiceMock, times(1)).publish("message", ""); } @@ -225,6 +240,8 @@ public void testReplyBroadcast() throws Exception { when(Jenkins.get()).thenReturn(jenkins); + PowerMockito.when(CredentialsObtainer.getTokenToUse(eq("globalTokenCredentialId"), any(Item.class), anyString())).thenReturn("token"); + when(stepContextMock.get(Project.class)).thenReturn(project); when(slackDescMock.getBaseUrl()).thenReturn("globalBaseUrl"); @@ -236,10 +253,10 @@ public void testReplyBroadcast() throws Exception { when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), eq(project))).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), anyString())).thenReturn(slackServiceMock); stepExecution.run(); - verify(stepExecution, times(1)).getSlackService("globalBaseUrl", "globalTeamDomain", null, "globalTokenCredentialId", false, "globalChannel", true, project); + verify(stepExecution, times(1)).getSlackService("globalBaseUrl", "globalTeamDomain", false, "globalChannel", true, "token"); verify(slackServiceMock, times(1)).publish("message", ""); } @@ -250,12 +267,15 @@ public void testNonNullEmptyColor() throws Exception { step.setColor(""); SlackSendStep.SlackSendStepExecution stepExecution = spy(new SlackSendStep.SlackSendStepExecution(step, stepContextMock)); + when(Jenkins.get()).thenReturn(jenkins); + PowerMockito.when(CredentialsObtainer.getTokenToUse(anyString(), any(Item.class), anyString())).thenReturn("token"); + when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), isNull(Item.class))).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), anyString())).thenReturn(slackServiceMock); stepExecution.run(); verify(slackServiceMock, times(1)).publish("message", ""); @@ -277,10 +297,12 @@ public void testSlackResponseObject() throws Exception { when(Jenkins.get()).thenReturn(jenkins); + PowerMockito.when(CredentialsObtainer.getTokenToUse(anyString(), any(Item.class), anyString())).thenReturn("token"); + when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), isNull(Item.class))).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), anyString())).thenReturn(slackServiceMock); String savedResponse = IOUtils.toString( this.getClass().getResourceAsStream("response.json") @@ -315,10 +337,12 @@ public void testSlackResponseObjectNullNonBotUser() throws Exception { when(Jenkins.get()).thenReturn(jenkins); + PowerMockito.when(CredentialsObtainer.getTokenToUse(eq("tokenCredentialId"), any(Item.class), anyString())).thenReturn("token"); + when(taskListenerMock.getLogger()).thenReturn(printStreamMock); doNothing().when(printStreamMock).println(); - when(stepExecution.getSlackService(anyString(), anyString(), anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), isNull(Item.class))).thenReturn(slackServiceMock); + when(stepExecution.getSlackService(anyString(), anyString(), anyBoolean(), anyString(), anyBoolean(), anyString())).thenReturn(slackServiceMock); when(slackServiceMock.getResponseString()).thenReturn(null); when(slackServiceMock.publish(anyString(), anyString())).thenReturn(true); From a01335b9fda9b00f80f005e33d3db33a99b466c2 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Fri, 15 Mar 2019 18:28:16 +0100 Subject: [PATCH 05/10] Update src/main/java/jenkins/plugins/slack/StandardSlackService.java Update IllegalArgumentException message Co-Authored-By: sirstrahd --- src/main/java/jenkins/plugins/slack/StandardSlackService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/jenkins/plugins/slack/StandardSlackService.java b/src/main/java/jenkins/plugins/slack/StandardSlackService.java index 7d241aba..e44ea8c5 100755 --- a/src/main/java/jenkins/plugins/slack/StandardSlackService.java +++ b/src/main/java/jenkins/plugins/slack/StandardSlackService.java @@ -90,7 +90,7 @@ public StandardSlackService(String baseUrl, String teamDomain, String token, Str public StandardSlackService(String baseUrl, String teamDomain, boolean botUser, String roomId, boolean replyBroadcast, String populatedToken) { this(baseUrl, teamDomain, botUser, roomId, replyBroadcast); if (populatedToken == null) { - throw new IllegalArgumentException("populatedToken cannot be null"); + throw new IllegalArgumentException("No slack token found, setup a secret text credential and configure it to be used"); } this.populatedToken = populatedToken; } From 94811cd49f32b5955ef792979ba24f0632b8384b Mon Sep 17 00:00:00 2001 From: Marc Salles Date: Fri, 15 Mar 2019 19:37:33 +0100 Subject: [PATCH 06/10] addresses PR comments --- .../plugins/slack/CredentialsObtainer.java | 23 +++++++---- .../jenkins/plugins/slack/SlackNotifier.java | 23 ++++++----- .../plugins/slack/StandardSlackService.java | 41 ++++++++----------- .../plugins/slack/workflow/SlackSendStep.java | 9 ++-- 4 files changed, 49 insertions(+), 47 deletions(-) diff --git a/src/main/java/jenkins/plugins/slack/CredentialsObtainer.java b/src/main/java/jenkins/plugins/slack/CredentialsObtainer.java index 88d23f37..972eae6b 100644 --- a/src/main/java/jenkins/plugins/slack/CredentialsObtainer.java +++ b/src/main/java/jenkins/plugins/slack/CredentialsObtainer.java @@ -2,6 +2,7 @@ import com.cloudbees.plugins.credentials.CredentialsMatcher; import com.cloudbees.plugins.credentials.CredentialsMatchers; +import com.cloudbees.plugins.credentials.CredentialsProvider; import hudson.model.Item; import hudson.security.ACL; import org.apache.commons.lang.StringUtils; @@ -13,7 +14,7 @@ public class CredentialsObtainer { public static StringCredentials lookupCredentials(String credentialId, Item item) { - List credentials = com.cloudbees.plugins.credentials.CredentialsProvider.lookupCredentials(StringCredentials.class, item, ACL.SYSTEM, Collections.emptyList()); + List credentials = CredentialsProvider.lookupCredentials(StringCredentials.class, item, ACL.SYSTEM, Collections.emptyList()); CredentialsMatcher matcher = CredentialsMatchers.withId(credentialId); return CredentialsMatchers.firstOrNull(credentials, matcher); } @@ -25,16 +26,20 @@ public static StringCredentials lookupCredentials(String credentialId, Item item * @param token the fallback token * @return the obtained token */ - public static String getTokenToUse(String credentialId, Item item, String token) { + public static String getTokenToUse(String credentialId, Item item, String token) { + String response; if (StringUtils.isEmpty(credentialId)) { - return token; - } - StringCredentials credentials = lookupCredentials(StringUtils.trim(credentialId), item); - final String response; - if (credentials != null) { - response = credentials.getSecret().getPlainText(); - } else { response = token; + } else { + StringCredentials credentials = lookupCredentials(StringUtils.trim(credentialId), item); + if (credentials != null) { + response = credentials.getSecret().getPlainText(); + } else { + response = token; + } + } + if (StringUtils.isEmpty(response)) { + throw new IllegalArgumentException("the token with the provided ID could not be found and no token was specified"); } return response; } diff --git a/src/main/java/jenkins/plugins/slack/SlackNotifier.java b/src/main/java/jenkins/plugins/slack/SlackNotifier.java index af858d5e..35d8e1fa 100755 --- a/src/main/java/jenkins/plugins/slack/SlackNotifier.java +++ b/src/main/java/jenkins/plugins/slack/SlackNotifier.java @@ -408,7 +408,7 @@ public BuildStepMonitor getRequiredMonitorService() { return BuildStepMonitor.NONE; } - public SlackService newSlackService(AbstractBuild r, BuildListener listener) { + public SlackService newSlackService(AbstractBuild abstractBuild, BuildListener listener) { DescriptorImpl descriptor = getDescriptor(); String teamDomain = Util.fixEmpty(this.teamDomain) != null ? this.teamDomain : descriptor.getTeamDomain(); String baseUrl = Util.fixEmpty(this.baseUrl) != null ? this.baseUrl : descriptor.getBaseUrl(); @@ -420,7 +420,7 @@ public SlackService newSlackService(AbstractBuild r, BuildListener listener) { EnvVars env; try { - env = r.getEnvironment(listener); + env = abstractBuild.getEnvironment(listener); } catch (Exception e) { listener.getLogger().println("Error retrieving environment vars: " + e.getMessage()); env = new EnvVars(); @@ -430,15 +430,8 @@ public SlackService newSlackService(AbstractBuild r, BuildListener listener) { authToken = env.expand(authToken); authTokenCredentialId = env.expand(authTokenCredentialId); room = env.expand(room); - final String populatedToken = CredentialsObtainer.getTokenToUse(authTokenCredentialId, r.getProject(), authToken); - if (populatedToken != null) { - return new StandardSlackService(baseUrl, teamDomain, botUser, room, false, populatedToken); - } else { - logger.log(Level.INFO, "No explicit token specified and could not obtain credentials for id: " + authTokenCredentialId); - return new StandardSlackService(baseUrl, teamDomain, authToken, authTokenCredentialId, botUser, room, false); - } - - + final String populatedToken = CredentialsObtainer.getTokenToUse(authTokenCredentialId, abstractBuild.getParent(), authToken); + return new StandardSlackService(baseUrl, teamDomain, botUser, room, false, populatedToken); } @Override @@ -614,6 +607,14 @@ public boolean configure(StaplerRequest req, JSONObject formData) { return true; } + /** + * @deprecated use {@link #getSlackService(String, String, String, boolean, String, Item)} instead} + */ + @Deprecated + SlackService getSlackService(final String baseUrl, final String teamDomain, final String authTokenCredentialId, final boolean botUser, final String roomId) { + return getSlackService(baseUrl, teamDomain, authTokenCredentialId, botUser, roomId, null); + } + SlackService getSlackService(final String baseUrl, final String teamDomain, final String authTokenCredentialId, final boolean botUser, final String roomId, final Item item) { final String populatedToken = CredentialsObtainer.getTokenToUse(authTokenCredentialId, item,null ); if (populatedToken != null) { diff --git a/src/main/java/jenkins/plugins/slack/StandardSlackService.java b/src/main/java/jenkins/plugins/slack/StandardSlackService.java index e44ea8c5..945aaba7 100755 --- a/src/main/java/jenkins/plugins/slack/StandardSlackService.java +++ b/src/main/java/jenkins/plugins/slack/StandardSlackService.java @@ -45,13 +45,11 @@ public class StandardSlackService implements SlackService { private String host = "slack.com"; private String baseUrl; private String teamDomain; - private String token = null; - private String authTokenCredentialId = null; private boolean botUser; private String[] roomIds; private boolean replyBroadcast; - private String responseString = null; - private String populatedToken = null; + private String responseString; + private String populatedToken; /** * @deprecated use {@link #StandardSlackService(String, String, boolean, String, boolean, String)} instead} @@ -75,8 +73,7 @@ public StandardSlackService(String baseUrl, String teamDomain, String token, Str @Deprecated public StandardSlackService(String baseUrl, String teamDomain, String token, String authTokenCredentialId, boolean botUser, String roomId, boolean replyBroadcast) { this(baseUrl, teamDomain, botUser, roomId, replyBroadcast); - this.token = token; - this.authTokenCredentialId = StringUtils.trim(authTokenCredentialId); + populatedToken = getTokenToUse(authTokenCredentialId, token); } /** @@ -103,12 +100,13 @@ private StandardSlackService(String baseUrl, String teamDomain, boolean botUser, } this.teamDomain = teamDomain; this.botUser = botUser; + if (roomId == null) { + throw new IllegalArgumentException("Project Channel or Slack User ID must be specified."); + } this.roomIds = roomId.split("[,; ]+"); this.replyBroadcast = replyBroadcast; } - - public String getResponseString() { return responseString; } @@ -156,12 +154,11 @@ public boolean publish(String message, JSONArray attachments, String color) { roomId = splitThread[0]; threadTs = splitThread[1]; } - final String token = getTokenToUse(); //prepare post methods for both requests types if (!botUser || !StringUtils.isEmpty(baseUrl)) { - url = "https://" + teamDomain + "." + host + "/services/hooks/jenkins-ci?token=" + token; + url = "https://" + teamDomain + "." + host + "/services/hooks/jenkins-ci?token=" + populatedToken; if (!StringUtils.isEmpty(baseUrl)) { - url = baseUrl + token; + url = baseUrl + populatedToken; } post = new HttpPost(url); JSONObject json = new JSONObject(); @@ -175,7 +172,7 @@ public boolean publish(String message, JSONArray attachments, String color) { nvps.add(new BasicNameValuePair("payload", json.toString())); } else { - url = "https://slack.com/api/chat.postMessage?token=" + token + + url = "https://slack.com/api/chat.postMessage?token=" + populatedToken + "&channel=" + roomId.replace("#", "") + "&link_names=1" + "&as_user=true"; @@ -221,20 +218,16 @@ public boolean publish(String message, JSONArray attachments, String color) { return result; } - private String getTokenToUse() { - if (populatedToken != null) { - return populatedToken; - } else { - if (!StringUtils.isEmpty(authTokenCredentialId)) { - StringCredentials credentials = CredentialsObtainer.lookupCredentials(authTokenCredentialId, null); - if (credentials != null) { - logger.fine("Using Integration Token Credential ID."); - return credentials.getSecret().getPlainText(); - } + private String getTokenToUse(String authTokenCredentialId, String token) { + if (!StringUtils.isEmpty(authTokenCredentialId)) { + StringCredentials credentials = CredentialsObtainer.lookupCredentials(authTokenCredentialId, null); + if (credentials != null) { + logger.fine("Using Integration Token Credential ID."); + return credentials.getSecret().getPlainText(); } - - logger.fine("Using Integration Token."); } + + logger.fine("Using Integration Token."); return token; } diff --git a/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java b/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java index db522c27..cab1921a 100644 --- a/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java +++ b/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java @@ -257,12 +257,15 @@ protected SlackResponse run() throws Exception { defaultIfEmpty(baseUrl), defaultIfEmpty(teamDomain), channel, defaultIfEmpty(color), botUser, defaultIfEmpty(tokenCredentialId)) ); - final String populatedToken = CredentialsObtainer.getTokenToUse(tokenCredentialId, item, token); - if (populatedToken == null) { + final String populatedToken; + try { + populatedToken = CredentialsObtainer.getTokenToUse(tokenCredentialId, item, token); + } catch (IllegalArgumentException e) { listener.error(Messages - .NotificationFailedWithException(new IllegalArgumentException("the token with the provided ID could not be found and no token was specified"))); + .NotificationFailedWithException(e)); return null; } + SlackService slackService = getSlackService( baseUrl, teamDomain, botUser, channel, step.replyBroadcast, populatedToken); final boolean publishSuccess; From 4e071dcb840a7992827fdb4afd0817a1d10c2ff9 Mon Sep 17 00:00:00 2001 From: Marc Salles Date: Fri, 15 Mar 2019 20:21:44 +0100 Subject: [PATCH 07/10] fix additional PR comments --- .../java/jenkins/plugins/slack/CredentialsObtainer.java | 7 +++++++ .../java/jenkins/plugins/slack/StandardSlackService.java | 2 +- .../java/jenkins/plugins/slack/workflow/SlackSendStep.java | 2 -- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/main/java/jenkins/plugins/slack/CredentialsObtainer.java b/src/main/java/jenkins/plugins/slack/CredentialsObtainer.java index 972eae6b..ae60ff84 100644 --- a/src/main/java/jenkins/plugins/slack/CredentialsObtainer.java +++ b/src/main/java/jenkins/plugins/slack/CredentialsObtainer.java @@ -5,6 +5,7 @@ import com.cloudbees.plugins.credentials.CredentialsProvider; import hudson.model.Item; import hudson.security.ACL; +import jenkins.model.Jenkins; import org.apache.commons.lang.StringUtils; import org.jenkinsci.plugins.plaincredentials.StringCredentials; @@ -13,6 +14,12 @@ public class CredentialsObtainer { + public static StringCredentials lookupCredentials(String credentialId) { + List credentials = CredentialsProvider.lookupCredentials(StringCredentials.class, Jenkins.get(), ACL.SYSTEM, Collections.emptyList()); + CredentialsMatcher matcher = CredentialsMatchers.withId(credentialId); + return CredentialsMatchers.firstOrNull(credentials, matcher); + } + public static StringCredentials lookupCredentials(String credentialId, Item item) { List credentials = CredentialsProvider.lookupCredentials(StringCredentials.class, item, ACL.SYSTEM, Collections.emptyList()); CredentialsMatcher matcher = CredentialsMatchers.withId(credentialId); diff --git a/src/main/java/jenkins/plugins/slack/StandardSlackService.java b/src/main/java/jenkins/plugins/slack/StandardSlackService.java index 945aaba7..9f4e3522 100755 --- a/src/main/java/jenkins/plugins/slack/StandardSlackService.java +++ b/src/main/java/jenkins/plugins/slack/StandardSlackService.java @@ -220,7 +220,7 @@ public boolean publish(String message, JSONArray attachments, String color) { private String getTokenToUse(String authTokenCredentialId, String token) { if (!StringUtils.isEmpty(authTokenCredentialId)) { - StringCredentials credentials = CredentialsObtainer.lookupCredentials(authTokenCredentialId, null); + StringCredentials credentials = CredentialsObtainer.lookupCredentials(authTokenCredentialId); if (credentials != null) { logger.fine("Using Integration Token Credential ID."); return credentials.getSecret().getPlainText(); diff --git a/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java b/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java index cab1921a..7b1fa18c 100644 --- a/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java +++ b/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java @@ -63,8 +63,6 @@ public class SlackSendStep extends Step { private Object attachments; private boolean replyBroadcast; - - @Nonnull public String getMessage() { return message; From 8217e7cf632622025d9649df8694f2912111f5b2 Mon Sep 17 00:00:00 2001 From: Marc Salles Date: Fri, 15 Mar 2019 22:07:50 +0100 Subject: [PATCH 08/10] Address PR comments + exception handling for factory calls --- .../plugins/slack/CredentialsObtainer.java | 12 ++- .../jenkins/plugins/slack/SlackNotifier.java | 25 +++-- .../plugins/slack/StandardSlackService.java | 5 +- .../StandardSlackServiceCredentialsTest.java | 91 ------------------- .../slack/StandardSlackServiceStub.java | 4 - .../slack/StandardSlackServiceTest.java | 35 ++++--- 6 files changed, 52 insertions(+), 120 deletions(-) delete mode 100644 src/test/java/jenkins/plugins/slack/StandardSlackServiceCredentialsTest.java diff --git a/src/main/java/jenkins/plugins/slack/CredentialsObtainer.java b/src/main/java/jenkins/plugins/slack/CredentialsObtainer.java index ae60ff84..2358f88d 100644 --- a/src/main/java/jenkins/plugins/slack/CredentialsObtainer.java +++ b/src/main/java/jenkins/plugins/slack/CredentialsObtainer.java @@ -11,19 +11,18 @@ import java.util.Collections; import java.util.List; +import java.util.NoSuchElementException; public class CredentialsObtainer { public static StringCredentials lookupCredentials(String credentialId) { List credentials = CredentialsProvider.lookupCredentials(StringCredentials.class, Jenkins.get(), ACL.SYSTEM, Collections.emptyList()); - CredentialsMatcher matcher = CredentialsMatchers.withId(credentialId); - return CredentialsMatchers.firstOrNull(credentials, matcher); + return getCredentialWithId(credentialId, credentials); } public static StringCredentials lookupCredentials(String credentialId, Item item) { List credentials = CredentialsProvider.lookupCredentials(StringCredentials.class, item, ACL.SYSTEM, Collections.emptyList()); - CredentialsMatcher matcher = CredentialsMatchers.withId(credentialId); - return CredentialsMatchers.firstOrNull(credentials, matcher); + return getCredentialWithId(credentialId, credentials); } /** @@ -50,4 +49,9 @@ public static String getTokenToUse(String credentialId, Item item, String token) } return response; } + + private static StringCredentials getCredentialWithId(String credentialId, List credentials) { + CredentialsMatcher matcher = CredentialsMatchers.withId(credentialId); + return CredentialsMatchers.firstOrNull(credentials, matcher); + } } diff --git a/src/main/java/jenkins/plugins/slack/SlackNotifier.java b/src/main/java/jenkins/plugins/slack/SlackNotifier.java index 35d8e1fa..28417068 100755 --- a/src/main/java/jenkins/plugins/slack/SlackNotifier.java +++ b/src/main/java/jenkins/plugins/slack/SlackNotifier.java @@ -445,20 +445,29 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListen BuildAwareLogger log = createLogger(listener); log.debug(buildKey, "Performing complete notifications"); JenkinsTokenExpander tokenExpander = new JenkinsTokenExpander(listener); - new ActiveNotifier(this, slackFactory(listener), log, tokenExpander).completed(build); - if (notifyRegression) { - log.debug(buildKey, "Performing finalize notifications"); - new ActiveNotifier(this, slackFactory(listener), log, tokenExpander).finalized(build); + try { + new ActiveNotifier(this, slackFactory(listener), log, tokenExpander).completed(build); + if (notifyRegression) { + log.debug(buildKey, "Performing finalize notifications"); + new ActiveNotifier(this, slackFactory(listener), log, tokenExpander).finalized(build); + } + } catch (Exception e) { + log.info(buildKey,"Exception attempting Slack notification: " + e.getMessage()); } return true; } @Override public boolean prebuild(AbstractBuild build, BuildListener listener) { - if (startNotification) { - BuildAwareLogger log = createLogger(listener); - log.debug(BuildKey.format(build), "Performing start notifications"); - new ActiveNotifier(this, slackFactory(listener), log, new JenkinsTokenExpander(listener)).started(build); + String buildKey = BuildKey.format(build); + BuildAwareLogger log = createLogger(listener); + try { + if (startNotification) { + log.debug(buildKey, "Performing start notifications"); + new ActiveNotifier(this, slackFactory(listener), log, new JenkinsTokenExpander(listener)).started(build); + } + } catch (Exception e) { + log.info(buildKey,"Exception attempting Slack notification: " + e.getMessage()); } return super.prebuild(build, listener); } diff --git a/src/main/java/jenkins/plugins/slack/StandardSlackService.java b/src/main/java/jenkins/plugins/slack/StandardSlackService.java index 9f4e3522..160fe1e3 100755 --- a/src/main/java/jenkins/plugins/slack/StandardSlackService.java +++ b/src/main/java/jenkins/plugins/slack/StandardSlackService.java @@ -73,7 +73,10 @@ public StandardSlackService(String baseUrl, String teamDomain, String token, Str @Deprecated public StandardSlackService(String baseUrl, String teamDomain, String token, String authTokenCredentialId, boolean botUser, String roomId, boolean replyBroadcast) { this(baseUrl, teamDomain, botUser, roomId, replyBroadcast); - populatedToken = getTokenToUse(authTokenCredentialId, token); + this.populatedToken = getTokenToUse(authTokenCredentialId, token); + if (this.populatedToken == null) { + throw new IllegalArgumentException("No slack token found, setup a secret text credential and configure it to be used"); + } } /** diff --git a/src/test/java/jenkins/plugins/slack/StandardSlackServiceCredentialsTest.java b/src/test/java/jenkins/plugins/slack/StandardSlackServiceCredentialsTest.java deleted file mode 100644 index a1b8e698..00000000 --- a/src/test/java/jenkins/plugins/slack/StandardSlackServiceCredentialsTest.java +++ /dev/null @@ -1,91 +0,0 @@ -package jenkins.plugins.slack; - -import com.cloudbees.plugins.credentials.CredentialsProvider; -import hudson.ExtensionList; -import hudson.security.ACL; -import hudson.util.Secret; -import jenkins.model.Jenkins; -import org.apache.http.HttpStatus; -import org.jenkinsci.plugins.plaincredentials.StringCredentials; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Matchers; -import org.mockito.Mock; -import org.powermock.api.mockito.PowerMockito; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - -import static org.junit.Assert.assertTrue; -import static org.powermock.api.mockito.PowerMockito.mock; -import static org.powermock.api.mockito.PowerMockito.when; - -@RunWith(PowerMockRunner.class) -@PrepareForTest({Jenkins.class, StandardSlackService.class, CredentialsProvider.class, Secret.class}) -public class StandardSlackServiceCredentialsTest { - @Mock - Jenkins jenkins; - - @Before - public void setUp() { - PowerMockito.mockStatic(Jenkins.class); - } - - @Test - public void populatedTokenIsUsed() { - final String populatedToken = "secret-text"; - final StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", true, "#room1:1528317530", populatedToken); - final CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); - httpClientStub.setHttpStatus(HttpStatus.SC_OK); - service.setHttpClient(httpClientStub); - service.publish("message"); - assertTrue(httpClientStub.getLastRequest().getURI().toString().contains(populatedToken)); - } - - @Test - public void globalCredentialByIdUsed() { - final String id = "cred-2id"; - final String secretText = "secret-2text"; - setupGlobalAvailableCredentialId(id, secretText); - final StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", null, id, true, "#room1:1528317530"); - final CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); - httpClientStub.setHttpStatus(HttpStatus.SC_OK); - service.setHttpClient(httpClientStub); - service.publish("message"); - assertTrue(httpClientStub.getLastRequest().getURI().toString().contains(secretText)); - } - - @Test - public void tokenIsUsed() { - final String token = "explicittoken"; - final StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", token, null, true, "#room1:1528317530"); - final CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); - httpClientStub.setHttpStatus(HttpStatus.SC_OK); - service.setHttpClient(httpClientStub); - service.publish("message"); - assertTrue(httpClientStub.getLastRequest().getURI().toString().contains(token)); - } - - private void setupGlobalAvailableCredentialId(final String id, final String secretText) { - final CredentialsProvider credentialsProvider = mock(CredentialsProvider.class); - final ExtensionList extensionList = mock(ExtensionList.class); - final List listProviders = new ArrayList<>(1); - final List credentialList = new ArrayList<>(); - final StringCredentials credentials = mock(StringCredentials.class); - final Secret secret = mock(Secret.class); - listProviders.add(credentialsProvider); - when(extensionList.iterator()).thenReturn(listProviders.iterator()); - when(Jenkins.getInstance()).thenReturn(jenkins); - when(jenkins.getExtensionList(CredentialsProvider.class)).thenReturn(extensionList); - when(credentials.getId()).thenReturn(id); - when(secret.getPlainText()).thenReturn(secretText); - when(credentials.getSecret()).thenReturn(secret); - credentialList.add(credentials); - when(credentialsProvider.getCredentials(Matchers.eq(StringCredentials.class), Matchers.eq(jenkins), Matchers.eq(ACL.SYSTEM), Matchers.eq(Collections.emptyList()))).thenReturn(credentialList); - } - -} diff --git a/src/test/java/jenkins/plugins/slack/StandardSlackServiceStub.java b/src/test/java/jenkins/plugins/slack/StandardSlackServiceStub.java index 1d2a3bc5..62eba477 100644 --- a/src/test/java/jenkins/plugins/slack/StandardSlackServiceStub.java +++ b/src/test/java/jenkins/plugins/slack/StandardSlackServiceStub.java @@ -4,10 +4,6 @@ public class StandardSlackServiceStub extends StandardSlackService { private CloseableHttpClientStub httpClientStub; - public StandardSlackServiceStub(String baseUrl, String teamDomain, String token, String tokenCredentialId, boolean botUser, String roomId) { - super(baseUrl, teamDomain, token, tokenCredentialId, botUser, roomId, false); - } - public StandardSlackServiceStub(String baseUrl, String teamDomain, boolean botUser, String roomId, String populatedToken) { super(baseUrl, teamDomain, botUser, roomId, false, populatedToken); } diff --git a/src/test/java/jenkins/plugins/slack/StandardSlackServiceTest.java b/src/test/java/jenkins/plugins/slack/StandardSlackServiceTest.java index aeb8a897..44cb1e58 100755 --- a/src/test/java/jenkins/plugins/slack/StandardSlackServiceTest.java +++ b/src/test/java/jenkins/plugins/slack/StandardSlackServiceTest.java @@ -13,7 +13,7 @@ public class StandardSlackServiceTest { */ @Test public void publishWithBadHostShouldNotRethrowExceptions() { - StandardSlackService service = new StandardSlackService("", "foo", "token", null, false, "#general"); + StandardSlackService service = new StandardSlackService("", "foo", false, "#general", false, "token"); service.setHost("hostvaluethatwillcausepublishtofail"); service.publish("message"); } @@ -23,7 +23,7 @@ public void publishWithBadHostShouldNotRethrowExceptions() { */ @Test public void invalidTeamDomainShouldFail() { - StandardSlackService service = new StandardSlackService("", "my", "token", null, false, "#general"); + StandardSlackService service = new StandardSlackService("", "my", false, "#general", false, "token"); service.publish("message"); } @@ -32,13 +32,13 @@ public void invalidTeamDomainShouldFail() { */ @Test public void invalidTokenShouldFail() { - StandardSlackService service = new StandardSlackService("", "tinyspeck", "token", null, false, "#general"); + StandardSlackService service = new StandardSlackService("", "tinyspeck", false, "#general", false, "token"); service.publish("message"); } @Test public void publishToASingleRoomSendsASingleMessage() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1"); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", false, "#room1", "token"); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); service.setHttpClient(httpClientStub); service.publish("message"); @@ -47,7 +47,7 @@ public void publishToASingleRoomSendsASingleMessage() { @Test public void publishToMultipleRoomsSendsAMessageToEveryRoom() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1,#room2,#room3"); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", false, "#room1,#room2,#room3", "token"); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); service.setHttpClient(httpClientStub); service.publish("message"); @@ -56,7 +56,7 @@ public void publishToMultipleRoomsSendsAMessageToEveryRoom() { @Test public void successfulPublishToASingleRoomReturnsTrue() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1"); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", false, "#room1", "token"); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); @@ -75,7 +75,7 @@ public void successfulPublishToSingleRoomWithProvidedTokenReturnsTrue() { @Test public void successfulPublishToMultipleRoomsReturnsTrue() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1,#room2,#room3"); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", false, "#room1,#room2,#room3", "token"); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); @@ -84,7 +84,7 @@ public void successfulPublishToMultipleRoomsReturnsTrue() { @Test public void failedPublishToASingleRoomReturnsFalse() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1"); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", false, "#room1", "token"); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_NOT_FOUND); service.setHttpClient(httpClientStub); @@ -93,7 +93,7 @@ public void failedPublishToASingleRoomReturnsFalse() { @Test public void singleFailedPublishToMultipleRoomsReturnsFalse() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1,#room2,#room3"); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain",false, "#room1,#room2,#room3", "token"); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setFailAlternateResponses(true); httpClientStub.setHttpStatus(HttpStatus.SC_OK); @@ -103,7 +103,7 @@ public void singleFailedPublishToMultipleRoomsReturnsFalse() { @Test public void publishToEmptyRoomReturnsTrue() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, ""); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", false, "", "token"); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); @@ -113,7 +113,7 @@ public void publishToEmptyRoomReturnsTrue() { @Test public void sendAsBotUserReturnsTrue() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, true, "#room1"); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", true, "#room1", "token"); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); @@ -122,10 +122,21 @@ public void sendAsBotUserReturnsTrue() { @Test public void sendAsBotUserInThreadReturnsTrue() { - StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, true, "#room1:1528317530"); + StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", true, "#room1:1528317530", "token"); CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); assertTrue(service.publish("message")); } + + @Test + public void populatedTokenIsUsed() { + final String populatedToken = "secret-text"; + final StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", true, "#room1:1528317530", populatedToken); + final CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); + httpClientStub.setHttpStatus(HttpStatus.SC_OK); + service.setHttpClient(httpClientStub); + service.publish("message"); + assertTrue(httpClientStub.getLastRequest().getURI().toString().contains(populatedToken)); + } } From 2edbd9d2fea5d9f7f4d3cd0813bca3515a0e1539 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Sat, 16 Mar 2019 12:00:50 +0000 Subject: [PATCH 09/10] Rerun CI From c63a66c66a0949687cdfa4d4e3643c665cd141e2 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Sat, 16 Mar 2019 18:53:13 +0000 Subject: [PATCH 10/10] Fix pipeline syntax not retrieving folder creds --- .../jenkins/plugins/slack/workflow/SlackSendStep.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java b/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java index 7b1fa18c..0d024dc6 100644 --- a/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java +++ b/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java @@ -194,12 +194,12 @@ public String getDisplayName() { return Messages.SlackSendStepDisplayName(); } - public ListBoxModel doFillTokenCredentialIdItems(@AncestorInPath Project project) { + public ListBoxModel doFillTokenCredentialIdItems(@AncestorInPath Item item) { Jenkins jenkins = Jenkins.get(); - if (project == null && !jenkins.hasPermission(Jenkins.ADMINISTER) || - project != null && !project.hasPermission(Item.EXTENDED_READ)) { + if (item == null && !jenkins.hasPermission(Jenkins.ADMINISTER) || + item != null && !item.hasPermission(Item.EXTENDED_READ)) { return new StandardListBoxModel(); } @@ -207,7 +207,7 @@ public ListBoxModel doFillTokenCredentialIdItems(@AncestorInPath Project project .withEmptySelection() .withAll(lookupCredentials( StringCredentials.class, - project, + item, ACL.SYSTEM, new HostnameRequirement("*.slack.com")) );