diff --git a/src/main/java/org/alfresco/repo/security/authentication/AuthenticationServiceImpl.java b/src/main/java/org/alfresco/repo/security/authentication/AuthenticationServiceImpl.java index 1d5f307d2c..d3e162593c 100644 --- a/src/main/java/org/alfresco/repo/security/authentication/AuthenticationServiceImpl.java +++ b/src/main/java/org/alfresco/repo/security/authentication/AuthenticationServiceImpl.java @@ -33,6 +33,7 @@ import org.alfresco.repo.management.subsystems.ActivateableBean; import org.alfresco.repo.security.authentication.AuthenticationComponent.UserNameValidationMode; import org.alfresco.repo.tenant.TenantContextHolder; import org.alfresco.service.cmr.security.PersonService; +import org.alfresco.util.GUID; import org.alfresco.util.Pair; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -42,7 +43,10 @@ public class AuthenticationServiceImpl extends AbstractAuthenticationService imp private Log logger = LogFactory.getLog(AuthenticationServiceImpl.class); AuthenticationComponent authenticationComponent; TicketComponent ticketComponent; - + + /** a serviceInstanceId identifying this unique instance */ + private String serviceInstanceId; + private String domain; private boolean allowsUserCreation = true; private boolean allowsUserDeletion = true; @@ -85,6 +89,8 @@ public class AuthenticationServiceImpl extends AbstractAuthenticationService imp public AuthenticationServiceImpl() { super(); + + this.serviceInstanceId = GUID.generate(); } public void setTicketComponent(TicketComponent ticketComponent) @@ -124,9 +130,10 @@ public class AuthenticationServiceImpl extends AbstractAuthenticationService imp TenantContextHolder.setTenantDomain(tenant); if (protectionEnabled) { - if (protectedUsersCache.get(userName) != null) + final String protectedUserKey = getProtectedUserKey(userName); + if (protectedUsersCache.get(protectedUserKey) != null) { - protectedUsersCache.remove(userName); + protectedUsersCache.remove(protectedUserKey); } } } @@ -148,7 +155,8 @@ public class AuthenticationServiceImpl extends AbstractAuthenticationService imp boolean isProtected = false; if (protectionEnabled) { - ProtectedUser protectedUser = protectedUsersCache.get(userName); + final String protectedUserKey = getProtectedUserKey(userName); + ProtectedUser protectedUser = protectedUsersCache.get(protectedUserKey); if (protectedUser != null) { long currentTimeStamp = System.currentTimeMillis(); @@ -168,7 +176,8 @@ public class AuthenticationServiceImpl extends AbstractAuthenticationService imp { if (protectionEnabled) { - ProtectedUser protectedUser = protectedUsersCache.get(userName); + final String protectedUserKey = getProtectedUserKey(userName); + ProtectedUser protectedUser = protectedUsersCache.get(protectedUserKey); if (protectedUser == null) { protectedUser = new ProtectedUser(userName); @@ -186,10 +195,18 @@ public class AuthenticationServiceImpl extends AbstractAuthenticationService imp } } } - protectedUsersCache.put(userName, protectedUser); + protectedUsersCache.put(protectedUserKey, protectedUser); } } + /** + * Creates a key by combining the service instance ID with the username. This are the type of keys maintained by protectedUsersCache map. + */ + public String getProtectedUserKey(String userName) + { + return serviceInstanceId + "@@" + userName; + } + public String getCurrentUserName() throws AuthenticationException { return authenticationComponent.getCurrentUserName(); diff --git a/src/main/resources/alfresco/caches.properties b/src/main/resources/alfresco/caches.properties index c629c244b0..876a8640d6 100644 --- a/src/main/resources/alfresco/caches.properties +++ b/src/main/resources/alfresco/caches.properties @@ -729,7 +729,7 @@ cache.authorizationCache.readBackupData=false cache.protectedUsersCache.maxItems=1000 cache.protectedUsersCache.timeToLiveSeconds=0 cache.protectedUsersCache.maxIdleSeconds=0 -cache.protectedUsersCache.cluster.type=fully-distributed +cache.protectedUsersCache.cluster.type=local cache.protectedUsersCache.backup-count=1 cache.protectedUsersCache.eviction-policy=LRU cache.protectedUsersCache.eviction-percentage=25 diff --git a/src/test/java/org/alfresco/repo/security/authentication/AuthenticationServiceImplTest.java b/src/test/java/org/alfresco/repo/security/authentication/AuthenticationServiceImplTest.java index a7a322287d..366979083f 100644 --- a/src/test/java/org/alfresco/repo/security/authentication/AuthenticationServiceImplTest.java +++ b/src/test/java/org/alfresco/repo/security/authentication/AuthenticationServiceImplTest.java @@ -34,6 +34,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.Map; +import static junit.framework.TestCase.assertNotNull; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; @@ -57,6 +58,7 @@ public class AuthenticationServiceImplTest private SimpleCache cache; private TicketComponent ticketComponent = mock(TicketComponent.class); private AuthenticationServiceImpl authService; + private AuthenticationServiceImpl authService2; private static final String USERNAME = "username"; private static final char[] PASSWORD = "password".toCharArray(); @@ -69,6 +71,11 @@ public class AuthenticationServiceImplTest authService.setTicketComponent(ticketComponent); cache = new MockCache<>(); authService.setProtectedUsersCache(cache); + + authService2 = new AuthenticationServiceImpl(); + authService2.setAuthenticationComponent(authenticationComponent); + authService2.setTicketComponent(ticketComponent); + authService2.setProtectedUsersCache(cache); } @Test @@ -104,7 +111,9 @@ public class AuthenticationServiceImplTest } verify(authenticationComponent, times(limit)).authenticate(USERNAME, PASSWORD); assertTrue("The user should be protected.", authService.isUserProtected(USERNAME)); - assertEquals("The number of recorded logins did not match.", attempts, cache.get(USERNAME).getNumLogins()); + + final String protectedUserKey = authService.getProtectedUserKey(USERNAME); + assertEquals("The number of recorded logins did not match.", attempts, cache.get(protectedUserKey).getNumLogins()); // test that the protection is still in place even if the password is correct doNothing().when(authenticationComponent).authenticate(USERNAME, PASSWORD); @@ -118,7 +127,7 @@ public class AuthenticationServiceImplTest // normal } verify(authenticationComponent, times(limit)).authenticate(USERNAME, PASSWORD); - assertEquals("The number of recorded logins did not match.", attempts + 1, cache.get(USERNAME).getNumLogins()); + assertEquals("The number of recorded logins did not match.", attempts + 1, cache.get(protectedUserKey).getNumLogins()); } @Test @@ -145,11 +154,13 @@ public class AuthenticationServiceImplTest } } assertTrue("The user should be protected.", authService.isUserProtected(USERNAME)); - assertEquals("The number of recorded logins did not match.", attempts, cache.get(USERNAME).getNumLogins()); + + final String protectedUserKey = authService.getProtectedUserKey(USERNAME); + assertEquals("The number of recorded logins did not match.", attempts, cache.get(protectedUserKey).getNumLogins()); Thread.sleep(timeLimit*1000 + 1); assertFalse("The user should not be protected any more.", authService.isUserProtected(USERNAME)); assertEquals("The number of recorded logins should stay the same after protection period ends.", - attempts, cache.get(USERNAME).getNumLogins()); + attempts, cache.get(protectedUserKey).getNumLogins()); doNothing().when(authenticationComponent).authenticate(USERNAME, PASSWORD); try @@ -161,9 +172,78 @@ public class AuthenticationServiceImplTest fail("An " + AuthenticationException.class.getName() + " should not be thrown."); } assertNull("The user should be removed from the cache after successful login.", - cache.get(USERNAME)); + cache.get(protectedUserKey)); } + @Test + public void testAuthChainWorksIfFirstAuthFails() throws Exception + { + int timeLimit = 1; + int attempts = 2; + authService.setProtectionPeriodSeconds(timeLimit); + authService.setProtectionLimit(attempts); + authService.setProtectionEnabled(true); + + authService2.setProtectionPeriodSeconds(timeLimit); + authService2.setProtectionLimit(attempts); + authService2.setProtectionEnabled(true); + + AuthenticationServiceImpl[] authenticationChain = {authService, authService2}; + + doThrow(new AuthenticationException("Bad password")) + .when(authenticationComponent).authenticate(USERNAME, PASSWORD); + + // Authentication fails on first run. + for (int i = 0; i < attempts; i++) { + for (AuthenticationServiceImpl authentication : authenticationChain) { + try { + authentication.authenticate(USERNAME, PASSWORD); + fail("An " + AuthenticationException.class.getName() + " should be thrown."); + } catch (AuthenticationException ae) { + // normal + } + } + } + + for (AuthenticationServiceImpl authentication : authenticationChain) + { + assertTrue("The user should be protected.", authentication.isUserProtected(USERNAME)); + } + + Thread.sleep(timeLimit*1000 + 1); + + for (AuthenticationServiceImpl authentication : authenticationChain) + { + assertFalse("The user should not be protected any more.", authentication.isUserProtected(USERNAME)); + } + + // Authentication always fails on first authentication service in the chain. + try + { + authenticationChain[0].authenticate(USERNAME, PASSWORD); + fail("An " + AuthenticationException.class.getName() + " should be thrown."); + } catch (AuthenticationException ae) { + // normal + } + + // Authentication should succeed on second authentication service in the chain. + doNothing().when(authenticationComponent).authenticate(USERNAME, PASSWORD); + try + { + authenticationChain[1].authenticate(USERNAME, PASSWORD); + } + catch (AuthenticationException ae) + { + fail("An " + AuthenticationException.class.getName() + " should not be thrown."); + } + + assertNotNull("The user should not be removed from the cache for the corresponding authorization service after a failed login.", + cache.get(authenticationChain[0].getProtectedUserKey(USERNAME))); + assertNull("The user should be removed from the cache for the corresponding authorization service after successful login.", + cache.get(authenticationChain[1].getProtectedUserKey(USERNAME))); + } + + @Test public void testProtectionDisabledBadPassword() {