diff --git a/src/main/java/jenkins/plugins/slack/user/EmailSlackUserIdResolver.java b/src/main/java/jenkins/plugins/slack/user/EmailSlackUserIdResolver.java index 078959ba4..cca2579f3 100644 --- a/src/main/java/jenkins/plugins/slack/user/EmailSlackUserIdResolver.java +++ b/src/main/java/jenkins/plugins/slack/user/EmailSlackUserIdResolver.java @@ -29,11 +29,16 @@ import hudson.tasks.MailAddressResolver; import java.io.IOException; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; +import java.util.function.Predicate; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.stream.Collectors; import java.util.stream.Stream; import org.apache.commons.lang.StringUtils; import org.apache.http.HttpEntity; @@ -91,44 +96,82 @@ public void setMailAddressResolvers(List mailAddressResolve this.mailAddressResolvers = mailAddressResolvers; } - protected String resolveUserId(User user) { - Optional userId = Optional.ofNullable(mailAddressResolvers) - .map(Collection::stream) - .orElseGet(Stream::empty) - .map(resolver -> { - try { - return resolver.findMailAddressFor(user); - } catch (Exception ex) { - LOGGER.log(Level.WARNING, String.format( - "The email resolver '%s' failed", resolver.getClass().getName()), ex); - return null; - } - }) - .filter(StringUtils::isNotEmpty) - .map(this::resolveUserIdForEmailAddress) - .filter(StringUtils::isNotEmpty) - .findAny(); + private String resolveUserEmail(User user){ + return Optional.ofNullable(mailAddressResolvers) + .map(Collection::stream) + .orElseGet(Stream::empty) + .map(resolver -> { + try { + String email = resolver.findMailAddressFor(user); + LOGGER.log(Level.FINEST, String.format( + "The email resolver '%s' resolved %s as %s", resolver.getClass().getName(), user.getId(), email)); + return email; + } catch (Exception ex) { + LOGGER.log(Level.WARNING, String.format( + "The email resolver '%s' failed", resolver.getClass().getName()), ex); + return null; + } + }) + .filter(StringUtils::isNotEmpty) + .findAny() + .orElse(null); + } + + protected SlackUserProperty fetchUserSlackProperty(User user) { + String userEmail = resolveUserEmail(user); + + SlackUserProperty userProperty = null; + if(userEmail != null){ + userProperty = resolveSlackPropertyFromEmailViaSlack(userEmail); + } // Return value can be null, so Optional.orElseGet(Supplier) doesn't work. - if (userId.isPresent()) { - return userId.get(); + if (userProperty != null) { + return userProperty; } else if (defaultMailAddressResolver != null){ - return resolveUserIdForEmailAddress(defaultMailAddressResolver.apply(user)); + return resolveSlackPropertyFromEmailViaSlack(defaultMailAddressResolver.apply(user)); } else { return null; } } public String resolveUserIdForEmailAddress(String emailAddress) { + SlackUserProperty userProperty = resolveSlackProperty(emailAddress); + if(userProperty == null){ + return null; + } + if(userProperty.getDisableNotifications()){ + return null; + } + return userProperty.getUserId(); + } + + private SlackUserProperty resolveSlackProperty(String emailAddress) { if (StringUtils.isEmpty(emailAddress)) { LOGGER.fine("Email address was empty"); return null; } + SlackUserProperty userProperty = resolveSlackPropertyFromEmailViaSlack(emailAddress); + if(userProperty == null) { + userProperty = resolveSlackPropertyFromEmailViaInferredUsername(emailAddress); + } + if(userProperty == null) { + userProperty = resolveSlackPropertyFromEmailViaUsers(emailAddress); + } + + return userProperty; + } + + private SlackUserProperty resolveSlackPropertyFromEmailViaSlack(String emailAddress){ if (StringUtils.isEmpty(authToken)) { LOGGER.fine("Auth token was empty"); return null; } + if (StringUtils.isEmpty(emailAddress)) { + LOGGER.fine("Auth token was empty"); + return null; + } String slackUserId = null; final String url = String.format(LOOKUP_BY_EMAIL_METHOD_URL_FORMAT, emailAddress); @@ -149,7 +192,84 @@ public String resolveUserIdForEmailAddress(String emailAddress) { } catch (IOException | ParseException | JSONException ex) { LOGGER.log(Level.WARNING, "Error getting userId from Slack", ex); } - return slackUserId; + + SlackUserProperty userProperty = null; + if(slackUserId != null){ + LOGGER.fine(String.format("Found Slack ID '%s' for email '%s'", slackUserId, emailAddress)); + userProperty = new SlackUserProperty(); + userProperty.setDisableNotifications(false); + userProperty.setUserId(slackUserId); + } else { + LOGGER.log(Level.INFO, String.format("Failed to resolve userId from Slack from email '%s'", emailAddress)); + } + return userProperty; + } + + private SlackUserProperty resolveSlackPropertyFromEmailViaInferredUsername(String emailAddress){ + if (StringUtils.isEmpty(emailAddress)) { + return null; + } + + String baseUsername = emailAddress.split("@")[0]; + + User user = User.get(baseUsername, false, Collections.emptyMap()); + if(user == null){ + LOGGER.log(Level.INFO, String.format("Could not find user with name '%s' from email '%s'", baseUsername, emailAddress)); + return null; + } + String userEmail = resolveUserEmail(user); + if(userEmail != null && !emailAddress.equals(userEmail)){ + LOGGER.log(Level.INFO, String.format("User with name '%s' does not match expected email '%s': have '%s'", baseUsername, emailAddress, userEmail)); + return null; + } + SlackUserProperty userProperty = user.getProperty(SlackUserProperty.class); + if(userProperty == null){ + LOGGER.log(Level.INFO, String.format("User with name '%s' does not have slack property", baseUsername)); + } else { + LOGGER.fine(String.format("Found Slack ID '%s' for user '%s'", userProperty.getUserId(), baseUsername)); + } + return userProperty; + } + + private SlackUserProperty resolveSlackPropertyFromEmailViaUsers(String emailAddress){ + if (StringUtils.isEmpty(emailAddress)) { + return null; + } + + List usersWithEmailAddress = User.getAll().stream() + .filter(user -> emailAddress.equals(resolveUserEmail(user))) + .collect(Collectors.toList()); + List usersPerId = usersWithEmailAddress.stream() + .map(user -> new ResolvedUserConfig(user)) + .filter(user -> user.slackProperty != null && user.slackProperty.getUserId() != null) + .filter(distinctSlackUserProperties()) + .collect(Collectors.toList()); + if(usersPerId.size() > 1) { + List conflictingUsersDisplayName = usersPerId.stream() + .map(resolved -> resolved.user.getDisplayName()) + .collect(Collectors.toList()); + LOGGER.log(Level.WARNING, String.format( + "Multiple users found with email '%s' having different slack IDs or configuration: %s", emailAddress, String.join(",", conflictingUsersDisplayName))); + } + Optional pickedUser = usersPerId.stream().findFirst(); + if(pickedUser.isPresent()) { + return pickedUser.get().slackProperty; + } + return null; + } + + private static class ResolvedUserConfig { + private User user; + private SlackUserProperty slackProperty; + public ResolvedUserConfig(User user) { + this.user = user; + this.slackProperty = user.getProperty(SlackUserProperty.class); + } + } + + public static Predicate distinctSlackUserProperties() { + Set seen = ConcurrentHashMap.newKeySet(); + return t -> seen.add(String.format("%s-%b", t.slackProperty.getUserId(), t.slackProperty.getDisableNotifications())); } @Extension diff --git a/src/main/java/jenkins/plugins/slack/user/NoSlackUserIdResolver.java b/src/main/java/jenkins/plugins/slack/user/NoSlackUserIdResolver.java index d6232d8de..23b70f1f5 100644 --- a/src/main/java/jenkins/plugins/slack/user/NoSlackUserIdResolver.java +++ b/src/main/java/jenkins/plugins/slack/user/NoSlackUserIdResolver.java @@ -34,7 +34,7 @@ public NoSlackUserIdResolver() { super(null, null); } - protected String resolveUserId(User user) { + protected SlackUserProperty fetchUserSlackProperty(User user) { return null; } diff --git a/src/main/java/jenkins/plugins/slack/user/SlackUserIdResolver.java b/src/main/java/jenkins/plugins/slack/user/SlackUserIdResolver.java index a7b0cf6bb..fb95f305e 100644 --- a/src/main/java/jenkins/plugins/slack/user/SlackUserIdResolver.java +++ b/src/main/java/jenkins/plugins/slack/user/SlackUserIdResolver.java @@ -58,18 +58,11 @@ protected SlackUserIdResolver(String authToken, CloseableHttpClient httpClient) } public final String findOrResolveUserId(User user) { - String userId = null; SlackUserProperty userProperty = user.getProperty(SlackUserProperty.class); - if (userProperty != null) { - userId = userProperty.getUserId(); - } else { - userProperty = new SlackUserProperty(); - } - if (StringUtils.isEmpty(userId)) { - userId = resolveUserId(user); - if (userId != null) { - userProperty.setUserId(userId); + if (userProperty == null || StringUtils.isEmpty(userProperty.getUserId())) { + userProperty = fetchUserSlackProperty(user); + if (userProperty.getUserId() != null) { try { user.addProperty(userProperty); } catch (IOException ex) { @@ -79,10 +72,10 @@ public final String findOrResolveUserId(User user) { } final boolean enableNotifications = !userProperty.getDisableNotifications(); - return enableNotifications ? userId : null; + return enableNotifications ? userProperty.getUserId() : null; } - protected abstract String resolveUserId(User user); + protected abstract SlackUserProperty fetchUserSlackProperty(User user); @SuppressWarnings("unchecked") public List resolveUserIdsForRun(Run run) { diff --git a/src/test/java/jenkins/plugins/slack/user/EmailSlackUserIdResolverTest.java b/src/test/java/jenkins/plugins/slack/user/EmailSlackUserIdResolverTest.java index d7b842f14..05d6db9d3 100644 --- a/src/test/java/jenkins/plugins/slack/user/EmailSlackUserIdResolverTest.java +++ b/src/test/java/jenkins/plugins/slack/user/EmailSlackUserIdResolverTest.java @@ -42,13 +42,20 @@ import org.junit.Test; import org.jvnet.hudson.test.FakeChangeLogSCM.EntryImpl; import org.jvnet.hudson.test.FakeChangeLogSCM.FakeChangeLogSet; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertNotNull; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -78,37 +85,92 @@ public void setUp() { } @Test - public void testResolveUserIdForEmailAddress() throws IOException { + public void testResolveUserIdForEmailAddressViaSlack() throws IOException { String userId; - // Test handling of a success response from Slack httpClient.setHttpResponse(getResponseOK()); userId = resolver.resolveUserIdForEmailAddress(EMAIL_ADDRESS); assertEquals(EXPECTED_USER_ID, userId); + } - // Test handling of an error response from Slack + @Test + public void testResolveUserIdFindByInferredUsername() throws IOException { + String userId; httpClient.setHttpResponse(getResponseError()); - userId = resolver.resolveUserIdForEmailAddress(EMAIL_ADDRESS); - assertNull(userId); + User mockUser = mock(User.class); + SlackUserProperty userProperty = new SlackUserProperty(); + userProperty.setUserId(EXPECTED_USER_ID); + when(mockUser.getProperty(SlackUserProperty.class)).thenReturn(userProperty); + try (MockedStatic userMock = Mockito.mockStatic(User.class)) { + userMock.when(() -> User.get(anyString(), anyBoolean(), any())).thenReturn(mockUser); + + userId = resolver.resolveUserIdForEmailAddress(EMAIL_ADDRESS); + + assertEquals(EXPECTED_USER_ID, userId); + verify(mailAddressResolver, times(1)).findMailAddressFor(mockUser); + userMock.verify(() -> User.get(eq("spengler"), eq(false), any()), times(1)); + } } @Test - public void testResolveUserIdForUser() throws Exception { - // MailAddressResolver is mocked to return EMAIL_ADDRESS associated with - // the EXPECTED_USER_ID - httpClient.setHttpResponse(getResponseOK()); - String userId = resolver.findOrResolveUserId(mock(User.class)); - assertEquals(EXPECTED_USER_ID, userId); + public void testResolveUserIdFindByInferredUsernameBadEmail() throws IOException { + String userId; + httpClient.setHttpResponse(getResponseError()); + User mockUser = mock(User.class); + SlackUserProperty userProperty = new SlackUserProperty(); + userProperty.setUserId(EXPECTED_USER_ID); + when(mockUser.getProperty(SlackUserProperty.class)).thenReturn(userProperty); + when(mailAddressResolver.findMailAddressFor(any(User.class))).thenReturn("nope@example.com"); + try (MockedStatic userMock = Mockito.mockStatic(User.class)) { + userMock.when(() -> User.get(anyString(), anyBoolean(), any())).thenReturn(mockUser); + + userId = resolver.resolveUserIdForEmailAddress(EMAIL_ADDRESS); + + assertNull(userId); + verify(mailAddressResolver, times(1)).findMailAddressFor(mockUser); + userMock.verify(() -> User.get(eq("spengler"), eq(false), any()), times(1)); + } } @Test - public void testResolveUserIdForUserWithoutEmailAddress() throws Exception { + public void testResolveUserIdFindByUsersListing() throws IOException { + String userId; + httpClient.setHttpResponse(getResponseError()); + User mockUser = mock(User.class); + SlackUserProperty userProperty = new SlackUserProperty(); + userProperty.setUserId(EXPECTED_USER_ID); + when(mockUser.getProperty(SlackUserProperty.class)).thenReturn(userProperty); + try (MockedStatic userMock = Mockito.mockStatic(User.class)) { + userMock.when(() -> User.get(anyString(), anyBoolean(), eq(null))).thenReturn(null); + userMock.when(() -> User.getAll()).thenReturn(Collections.singletonList(mockUser)); + + userId = resolver.resolveUserIdForEmailAddress(EMAIL_ADDRESS); + + assertEquals(EXPECTED_USER_ID, userId); + verify(mailAddressResolver, times(1)).findMailAddressFor(mockUser); + userMock.verify(() -> User.get(eq("spengler"), eq(false), any()), times(1)); + } + } + + @Test + public void testResolveUserIdForUserWithoutResolvableEmailAddress() throws Exception { mailAddressResolver = mock(MailAddressResolver.class); + resolver = new EmailSlackUserIdResolver(AUTH_TOKEN, httpClient, Collections.emptyList(), user -> null); + httpClient.setHttpResponse(getResponseOK()); + + SlackUserProperty userProperty = resolver.fetchUserSlackProperty(mock(User.class)); + assertNull(userProperty); + } + + @Test + public void testResolveUserIdForUserWithResolvableEmailAddressViaResolver() throws Exception { resolver = new EmailSlackUserIdResolver(AUTH_TOKEN, httpClient, Collections.singletonList(mailAddressResolver), user -> null); httpClient.setHttpResponse(getResponseOK()); - String userId = resolver.resolveUserId(mock(User.class)); - assertNull(userId); + SlackUserProperty userProperty = resolver.fetchUserSlackProperty(mock(User.class)); + assertNotNull(userProperty); + assertEquals(EXPECTED_USER_ID, userProperty.getUserId()); + assertEquals(false, userProperty.getDisableNotifications()); } @Test @@ -118,8 +180,8 @@ public void testResolveUserIdWithoutAuthToken() throws Exception { resolver.setAuthToken(null); httpClient.setHttpResponse(getResponseOK()); - String userId = resolver.resolveUserId(mock(User.class)); - assertNull(userId); + SlackUserProperty userProperty = resolver.fetchUserSlackProperty(mock(User.class)); + assertNull(userProperty); } @Test @@ -128,14 +190,23 @@ public void testResolveUserIdForUserWithoutDefaultMailAddressResolver() throws E resolver = new EmailSlackUserIdResolver(AUTH_TOKEN, httpClient, Collections.singletonList(mailAddressResolver), null); httpClient.setHttpResponse(getResponseOK()); - String userId = resolver.resolveUserId(mock(User.class)); - assertNull(userId); + SlackUserProperty userProperty = resolver.fetchUserSlackProperty(mock(User.class)); + assertNull(userProperty); + } + + @Test + public void testResolveUserIdForUser() throws Exception { + // MailAddressResolver is mocked to return EMAIL_ADDRESS associated with + // the EXPECTED_USER_ID + httpClient.setHttpResponse(getResponseOK()); + String userId = resolver.findOrResolveUserId(mock(User.class)); + assertEquals(EXPECTED_USER_ID, userId); } @Test public void testResolveUserIdForUserWithoutResolver() throws Exception { - resolver = new EmailSlackUserIdResolver(AUTH_TOKEN, httpClient, null, user -> {return EMAIL_ADDRESS;}); + resolver = new EmailSlackUserIdResolver(AUTH_TOKEN, httpClient, null, user -> EMAIL_ADDRESS); httpClient.setHttpResponse(getResponseOK()); String userId = resolver.findOrResolveUserId(mock(User.class));