Don't try to reset a user password that is already in use

src/main/java/de/aservo/confapi/crowd/service/UsersServiceImpl.java

@@ -1,8 +1,10 @@
 package de.aservo.confapi.crowd.service;
 
+import com.atlassian.crowd.embedded.api.CrowdService;
 import com.atlassian.crowd.embedded.api.Directory;
 import com.atlassian.crowd.embedded.api.PasswordCredential;
 import com.atlassian.crowd.exception.DirectoryNotFoundException;
+import com.atlassian.crowd.exception.FailedAuthenticationException;
 import com.atlassian.crowd.exception.InvalidCredentialException;
 import com.atlassian.crowd.exception.InvalidUserException;
 import com.atlassian.crowd.exception.OperationFailedException;
@@ -39,13 +41,18 @@
 @ExportAsService(UsersService.class)
 public class UsersServiceImpl implements UsersService {
 
+    @ComponentImport
+    private final CrowdService crowdService;
+
     @ComponentImport
     private final DirectoryManager directoryManager;
 
     @Inject
     public UsersServiceImpl(
+            final CrowdService crowdService,
             final DirectoryManager directoryManager) {
 
+        this.crowdService = crowdService;
         this.directoryManager = directoryManager;
     }
 
@@ -311,6 +318,19 @@ void updatePassword(
             final User user,
             final String password) {
 
+        // If the password is the same as the current one, do nothing,
+        // to avoid clashing with Crowd's password history count
+        try {
+            final com.atlassian.crowd.embedded.api.User authenticatedUser = crowdService.authenticate(user.getName(), password);
+
+            // The null check is unnecessary (return would be enough), but it simplifies mocking in tests
+            if (authenticatedUser != null) {
+                return;
+            }
+        } catch (FailedAuthenticationException e) {
+            // Ignore - if the password is wrong, we can actually try to update it
+        }
+
         try {
             directoryManager.updateUserCredential(user.getDirectoryId(), user.getName(), PasswordCredential.unencrypted(password));
         } catch (DirectoryPermissionException | InvalidCredentialException e) {

src/test/java/de/aservo/confapi/crowd/service/UsersServiceTest.java

@@ -1,5 +1,6 @@
 package de.aservo.confapi.crowd.service;
 
+import com.atlassian.crowd.embedded.api.CrowdService;
 import com.atlassian.crowd.embedded.api.Directory;
 import com.atlassian.crowd.embedded.api.MockDirectoryInternal;
 import com.atlassian.crowd.embedded.api.PasswordCredential;
@@ -42,14 +43,17 @@
 @RunWith(MockitoJUnitRunner.class)
 public class UsersServiceTest {
 
+    @Mock
+    private CrowdService crowdService;
+
     @Mock
     private DirectoryManager directoryManager;
 
     private UsersServiceImpl usersService;
 
     @Before
     public void setup() {
-        usersService = new UsersServiceImpl(directoryManager);
+        usersService = new UsersServiceImpl(crowdService, directoryManager);
 
         setupDirectoryManager();
     }
@@ -344,6 +348,21 @@ public void testChangePassword() throws CrowdException, PermissionException {
         assertEquals(password, passwordCredentialArgumentCaptor.getValue().getCredential());
     }
 
+    @Test
+    public void testChangePasswordWithSamePassword() throws CrowdException, PermissionException {
+        final User user = getTestUser();
+        doReturn(user).when(directoryManager).findUserByName(user.getDirectoryId(), user.getName());
+
+        final String password = "pa55w0rd";
+        final com.atlassian.crowd.embedded.api.User authenticatedUserMock = mock(com.atlassian.crowd.embedded.api.User.class);
+        when(authenticatedUserMock.getName()).thenReturn(user.getName());
+        when(crowdService.getUser(user.getName())).thenReturn(authenticatedUserMock);
+        when(crowdService.authenticate(user.getName(), password)).thenReturn(authenticatedUserMock);
+
+        usersService.updatePassword(user.getName(), password);
+        verify(directoryManager, never()).updateUserCredential(anyLong(), anyString(), any());
+    }
+
     // We kind of need to test all the exceptions here, but it's also pointless to test
     // all the exact mappings, because that's like repeating the implementation.
     // For this reason, we are just using WebApplicationException as a catch-all for now.