diff --git a/src/main/java/de/aservo/confapi/crowd/service/UsersServiceImpl.java b/src/main/java/de/aservo/confapi/crowd/service/UsersServiceImpl.java index a6e7bd5..77a00f9 100644 --- a/src/main/java/de/aservo/confapi/crowd/service/UsersServiceImpl.java +++ b/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 mechanisms + 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) { diff --git a/src/test/java/de/aservo/confapi/crowd/service/UsersServiceTest.java b/src/test/java/de/aservo/confapi/crowd/service/UsersServiceTest.java index 735c83e..1643391 100644 --- a/src/test/java/de/aservo/confapi/crowd/service/UsersServiceTest.java +++ b/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,6 +43,9 @@ @RunWith(MockitoJUnitRunner.class) public class UsersServiceTest { + @Mock + private CrowdService crowdService; + @Mock private DirectoryManager directoryManager; @@ -49,7 +53,7 @@ public class UsersServiceTest { @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.