#532 Only change password if it has actually changed

Closes #532

Author: ghenzler

accesscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/authorizableinstaller/impl/AuthorizableInstallerServiceImpl.java

@@ -25,6 +25,7 @@
 import javax.jcr.Node;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
+import javax.jcr.SimpleCredentials;
 import javax.jcr.UnsupportedRepositoryOperationException;
 import javax.jcr.ValueFactory;
 
@@ -43,6 +44,7 @@
 import org.apache.sling.api.resource.Resource;
 import org.apache.sling.api.resource.ResourceResolver;
 import org.apache.sling.api.resource.ResourceResolverFactory;
+import org.apache.sling.jcr.api.SlingRepository;
 import org.apache.sling.jcr.resource.JcrResourceConstants;
 import org.osgi.service.component.annotations.Reference;
 import org.osgi.service.component.annotations.ReferenceCardinality;
@@ -99,6 +101,9 @@ public class AuthorizableInstallerServiceImpl implements
     @Reference(policyOption = ReferencePolicyOption.GREEDY)
     ResourceResolverFactory resourceResolverFactory;
 
+    @Reference(policyOption = ReferencePolicyOption.GREEDY)
+    SlingRepository repository;
+    
     @Override
     public void installAuthorizables(
             AcConfiguration acConfiguration,
@@ -155,7 +160,7 @@ private void installAuthorizableConfigurationBean(final Session session,
             // update password for users
             if (!authorizableToInstall.isGroup() && !authorizableConfigBean.isSystemUser()
                     && StringUtils.isNotBlank(authorizableConfigBean.getPassword())) {
-                setUserPassword(authorizableConfigBean, (User) authorizableToInstall);
+                setUserPassword(authorizableConfigBean, (User) authorizableToInstall, installLog);
             }
 
             // move authorizable if path changed (retaining existing members)
@@ -226,11 +231,25 @@ private void installKeys(Map<String, Key> keys, String userId, ResourceResolver
         }
     }
 
-
     void setUserPassword(final AuthorizableConfigBean authorizableConfigBean,
-                             final User authorizableToInstall) throws RepositoryException, AuthorizableCreatorException {
+            final User authorizableToInstall, InstallationLogger installLog) throws RepositoryException, AuthorizableCreatorException {
+
+        String userId = authorizableToInstall.getID();
         String password = getPassword(authorizableConfigBean);
-        authorizableToInstall.changePassword(password);
+        Session sessionForUser = null;
+        try {
+            sessionForUser = repository.login(new SimpleCredentials(userId, password.toCharArray()));
+            LOG.trace("Could obtain session {} for user {}, will not update password", sessionForUser, userId);
+            installLog.addVerboseMessage(LOG, "Password of user " + userId + " has not changed");
+        } catch (javax.jcr.LoginException e) {
+            LOG.trace("User {} could not log in with existing password", userId, e);
+            authorizableToInstall.changePassword(password);
+            installLog.addMessage(LOG, "Changed password of user " + userId);
+        } finally {
+            if (sessionForUser != null) {
+                sessionForUser.logout();
+            }
+        }
     }
 
     private String getPassword(final AuthorizableConfigBean authorizableConfigBean)

accesscontroltool-bundle/src/test/java/biz/netcentric/cq/tools/actool/authorizableinstaller/impl/AuthorizableInstallerServiceImplTest.java

@@ -14,20 +14,22 @@
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.verifyZeroInteractions;
+import static org.mockito.MockitoAnnotations.initMocks;
 
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.HashSet;
 import java.util.Set;
 
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
+import javax.jcr.SimpleCredentials;
 import javax.jcr.Value;
 import javax.jcr.ValueFactory;
 
@@ -36,17 +38,17 @@
 import org.apache.jackrabbit.api.security.user.Group;
 import org.apache.jackrabbit.api.security.user.User;
 import org.apache.jackrabbit.api.security.user.UserManager;
+import org.apache.sling.jcr.api.SlingRepository;
 import org.hamcrest.BaseMatcher;
 import org.hamcrest.Description;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Enclosed;
-import org.junit.runners.Parameterized;
+import org.mockito.InjectMocks;
 import org.mockito.Matchers;
 import org.mockito.Mock;
 import org.mockito.Mockito;
-import org.mockito.MockitoAnnotations;
 import org.mockito.Spy;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.runners.MockitoJUnitRunner;
@@ -59,6 +61,7 @@
 import biz.netcentric.cq.tools.actool.configmodel.AuthorizableConfigBean;
 import biz.netcentric.cq.tools.actool.configmodel.GlobalConfiguration;
 import biz.netcentric.cq.tools.actool.crypto.DecryptionService;
+import biz.netcentric.cq.tools.actool.history.InstallationLogger;
 import biz.netcentric.cq.tools.actool.history.PersistableInstallationLogger;
 
 @RunWith(Enclosed.class)
@@ -300,6 +303,7 @@ public void describeTo(Description desc) {
 
     public static final class SetUserPassword {
 
+        private static final String USER_ID = "userid";
         private static final String UNPROTECTED_PASSWORD = "unprotected_pass";
 
         @Mock
@@ -308,26 +312,48 @@ public static final class SetUserPassword {
         @Mock
         private DecryptionService decryptionService;
 
+        @Mock
+        private InstallationLogger installationLogger;
+
+        @Mock
+        private SlingRepository repository;
+
+        @Mock
+        private Session session;
+
+        @Spy
+        @InjectMocks
         private AuthorizableInstallerServiceImpl service;
 
+        private AuthorizableConfigBean configBean;
+
         @Before
-        public void setUp() throws CryptoException {
-            MockitoAnnotations.initMocks(this);
+        public void setUp() throws CryptoException, RepositoryException {
+            initMocks(this);
 
-            service = new AuthorizableInstallerServiceImpl();
-            service.decryptionService = decryptionService;
+            doReturn(USER_ID).when(user).getID();
 
             doReturn(UNPROTECTED_PASSWORD).when(decryptionService).decrypt(anyString());
+
+            configBean = new AuthorizableConfigBean();
+            configBean.setPassword("{some_protected_pass1}");
         }
 
         @Test
-        public void test() throws RepositoryException, AuthorizableCreatorException {
-            final AuthorizableConfigBean bean = new AuthorizableConfigBean();
-            bean.setPassword("{some_protected_pass1}");
+        public void testPasswordExists() throws RepositoryException, AuthorizableCreatorException {
 
-            service.setUserPassword(bean, user);
+            doReturn(session).when(repository).login(any(SimpleCredentials.class));
+            service.setUserPassword(configBean, user, installationLogger);
+            verify(user, times(0)).changePassword(anyString());
+        }
 
-            verify(user).changePassword(eq(UNPROTECTED_PASSWORD));
+        @Test
+        public void testPasswordDifferent() throws RepositoryException, AuthorizableCreatorException {
+            final AuthorizableConfigBean bean = new AuthorizableConfigBean();
+            bean.setPassword("{some_protected_pass1}");
+            doThrow(javax.jcr.LoginException.class).when(repository).login(any(SimpleCredentials.class));
+            service.setUserPassword(configBean, user, installationLogger);
+            verify(user, times(1)).changePassword(UNPROTECTED_PASSWORD);
         }
     }
 }