Add account lockout after repeated failed logins

dotCMS/src/main/java/com/dotmarketing/business/UserAPIImpl.java

@@ -1,5 +1,6 @@
 package com.dotmarketing.business;
 
+import com.dotcms.auth.LoginLockout;
 import com.dotcms.business.CloseDBIfOpened;
 import com.dotcms.business.WrapInTransaction;
 import com.dotcms.enterprise.PasswordFactoryProxy;
@@ -494,8 +495,13 @@ public void save(User userToSave, User user, boolean validatePassword,
                 Logger.error(UserAPIImpl.class, "An error occurred generating the hashed password for userId: " + userToSave.getUserId(), e);
                 throw new DotDataException("An error occurred generating the hashed password.");
             }
+            userToSave.setFailedLoginAttempts(0);
         }
         save(userToSave, user, respectFrontEndRoles);
+        if (validatePassword) {
+            // A new password set by an administrator (or the user) unlocks an account locked out by failed logins
+            LoginLockout.getInstance().clear(userToSave.getUserId());
+        }
     }
 
     @Override
@@ -680,7 +686,10 @@ public void updatePassword(User user, String newpass, User currentUser, boolean
         user.setIcqId("");
         user.setPasswordReset(GetterUtil.getBoolean(
                 PropsUtil.get(PropsUtil.PASSWORDS_CHANGE_ON_FIRST_USE)));
+        user.setFailedLoginAttempts(0);
         save(user, currentUser, respectFrontEndRoles);
+        // A new password (reset token flow or set by an administrator) unlocks an account locked out by failed logins
+        LoginLockout.getInstance().clear(user.getUserId());
 
     }
 

dotCMS/src/main/java/com/dotmarketing/business/UserFactoryImpl.java

@@ -927,5 +927,8 @@ public void delete(final User userToDelete) throws DotDataException {
         dotConnect.setSQL("delete from user_ where userid = ?");
         dotConnect.addParam(userToDelete.getUserId());
         dotConnect.loadResult();
+        // Login lockout state (see com.dotcms.auth.LoginLockout) belongs to the user and goes with them
+        new DotConnect().setSQL("delete from login_lockout where user_id = ?")
+                .addParam(userToDelete.getUserId()).loadResult();
     }
 }

dotCMS/src/main/java/com/dotmarketing/cms/login/factories/LoginFactory.java

@@ -1,5 +1,6 @@
 package com.dotmarketing.cms.login.factories;
 
+import com.dotcms.auth.LoginLockout;
 import com.dotcms.cms.login.PreventSessionFixationUtil;
 import com.dotcms.concurrent.lock.DotKeyLockManager;
 import com.dotcms.concurrent.lock.DotKeyLockManagerBuilder;
@@ -191,7 +192,8 @@ public static boolean doLogin(String userName, String password, boolean remember
 					auth = Authenticator.SUCCESS;
 				}
 
-    			match = auth == Authenticator.SUCCESS;
+    			match = auth == Authenticator.SUCCESS
+    					&& (skipPasswordCheck || !isLockedOut(user, LoginLockout.clientAddress(request)));
         	} else {
 	            if (comp.getAuthType().equals(Company.AUTH_TYPE_EA)) {
 	            	user = APILocator.getUserAPI().loadByUserByEmail(userName, APILocator.getUserAPI().getSystemUser(), false);
@@ -213,8 +215,10 @@ public static boolean doLogin(String userName, String password, boolean remember
 	            }
 	            match = true;
 	            if (!skipPasswordCheck) {
-	            // Validate password and rehash when is needed
-	            match = passwordMatch(password, user);
+	            // Validate password and rehash when is needed. Refused while a lockout applies to this
+	            // address or account; a failure counts toward the lock.
+	            match = LoginLockout.getInstance().passwordMatch(password, user,
+	            		LoginLockout.clientAddress(request), (secret, candidate) -> false);
 	            }
 
 	            if (match) {
@@ -225,9 +229,6 @@ public static boolean doLogin(String userName, String password, boolean remember
 
 	            } else {
 
-					match = false;
-					user.setFailedLoginAttempts(user.getFailedLoginAttempts()+1);
-					APILocator.getUserAPI().save(user,APILocator.getUserAPI().getSystemUser(),false);
 					SecurityLogger.logInfo(LoginFactory.class,"An invalid attempt to login as " + userName + " from IP: " + request.getRemoteAddr());
 
 	            }
@@ -267,6 +268,15 @@ public static boolean doLogin(String userName, String password, boolean remember
         return false;
     }
 
+	/**
+	 * LDAP checked the password; a login that a lockout applies to (this address, or the whole account)
+	 * is still refused.
+	 */
+	private static boolean isLockedOut(final User user, final String source) {
+		return null != user
+				&& LoginLockout.getInstance().check(user.getUserId(), source) == LoginLockout.Status.LOCKED;
+	}
+
 	private static void syncPassword(User user) {
 		try {
 
@@ -364,7 +374,7 @@ public static boolean doLogin(String userName, String password) throws NoSuchUse
     				Logger.debug(LoginFactory.class, "syncPassword not set or unable to load user", e);
     			}
 
-    			match = auth == Authenticator.SUCCESS;
+    			match = auth == Authenticator.SUCCESS && !isLockedOut(user, LoginLockout.clientAddress());
         	} else {
         	    // Liferay authentication
  	            if (comp.getAuthType().equals(Company.AUTH_TYPE_EA)) {
@@ -386,11 +396,8 @@ public static boolean doLogin(String userName, String password) throws NoSuchUse
  	            	return false;
  	            }
 
-                match = passwordMatch(password, user);
-                if (match == false) {
-                    user.setFailedLoginAttempts(user.getFailedLoginAttempts() + 1);
-                    APILocator.getUserAPI().save(user, APILocator.getUserAPI().getSystemUser(), false);
-                }
+                // Refused while a lockout applies to this address or account; a failure counts toward the lock.
+                match = LoginLockout.getInstance().passwordMatch(password, user);
         	}
 
             // if passwords match

dotCMS/src/main/java/com/dotmarketing/startup/runonce/Task261015CreateLoginLockoutTable.java

@@ -0,0 +1,53 @@
+package com.dotmarketing.startup.runonce;
+
+import com.dotmarketing.common.db.DotDatabaseMetaData;
+import com.dotmarketing.db.DbConnectionFactory;
+import com.dotmarketing.startup.AbstractJDBCStartupTask;
+import com.dotmarketing.util.Logger;
+
+import java.sql.SQLException;
+
+/**
+ * Creates {@code login_lockout}, which holds login attempt counts and locks for
+ * {@link com.dotcms.auth.LoginLockout}: one row per account and source (client address, or WebDAV from
+ * that address), plus one account-wide row (source {@code *}). A separate table keeps the migration
+ * additive: the {@code user_} schema does not change (only its failed-login counts restart at zero), and
+ * an older release rolled back onto this schema never reads the new table.
+ */
+public class Task261015CreateLoginLockoutTable extends AbstractJDBCStartupTask {
+
+    private static final String TABLE_NAME = "login_lockout";
+
+    /**
+     * Runs when the table is missing, or when it has the earlier per-account layout (no {@code source}
+     * column) from a pre-release build. Lockout state is short-lived, so that table is simply rebuilt.
+     * It also restarts every user's failed-login count: that count only reset on a successful login, so
+     * dormant accounts can carry large values, and it sets the delay applied to the next failed login.
+     */
+    @Override
+    public boolean forceRun() {
+        try {
+            final DotDatabaseMetaData metaData = new DotDatabaseMetaData();
+            return !metaData.tableExists(DbConnectionFactory.getConnection(), TABLE_NAME)
+                    || !metaData.hasColumn(TABLE_NAME, "source");
+        } catch (final SQLException e) {
+            Logger.error(this, e.getMessage(), e);
+            return false;
+        }
+    }
+
+    @Override
+    public String getPostgresScript() {
+        return "DROP TABLE IF EXISTS login_lockout;" // nosemgrep: gitlab.find_sec_bugs.CUSTOM_INJECTION-2 -- fully hardcoded DDL, no user input
+                + "CREATE TABLE login_lockout ("
+                + " user_id varchar(100) not null,"
+                + " source varchar(100) not null,"
+                + " failures int not null,"
+                + " window_start bigint not null,"
+                + " locked_until bigint not null,"
+                + " mod_date timestamptz not null,"
+                + " constraint pk_login_lockout primary key (user_id, source)"
+                + ");"
+                + "UPDATE user_ SET failedloginattempts = 0 WHERE failedloginattempts <> 0;";
+    }
+}

dotCMS/src/main/java/com/dotmarketing/util/TaskLocatorUtil.java

@@ -272,6 +272,7 @@
 import com.dotmarketing.startup.runonce.Task260420AddDotAuthPortletToMenu;
 import com.dotmarketing.startup.runonce.Task260720AddDefaultBaseTypeToFolderTable;
 import com.dotmarketing.startup.runonce.Task260826CreateSystemEventCursorTable;
+import com.dotmarketing.startup.runonce.Task261015CreateLoginLockoutTable;
 import com.google.common.collect.ImmutableList;
 
 import java.util.ArrayList;
@@ -617,6 +618,7 @@ public static List<Class<?>> getStartupRunOnceTaskClasses() {
         .add(Task260615AlterClusterIdLength.class)
         .add(Task260720AddDefaultBaseTypeToFolderTable.class)
         .add(Task260826CreateSystemEventCursorTable.class)
+        .add(Task261015CreateLoginLockoutTable.class)
         .build();
 
         return ret.stream().sorted(classNameComparator).collect(Collectors.toList());

dotCMS/src/main/java/com/dotmarketing/webdav/DotWebdavHelper.java

@@ -4,6 +4,7 @@
 import static com.dotmarketing.business.PermissionAPI.PERMISSION_READ;
 
 import com.dotcms.api.web.HttpServletRequestThreadLocal;
+import com.dotcms.auth.LoginLockout;
 import com.dotcms.auth.providers.jwt.beans.JWToken;
 import com.dotcms.rendering.velocity.services.DotResourceCache;
 import com.bradmcevoy.http.CollectionResource;
@@ -29,7 +30,6 @@
 import com.dotmarketing.business.UserAPI;
 import com.dotmarketing.business.Versionable;
 import com.dotmarketing.cache.FolderCache;
-import com.dotmarketing.cms.login.factories.LoginFactory;
 import com.dotmarketing.exception.DotDataException;
 import com.dotmarketing.exception.DotRuntimeException;
 import com.dotmarketing.exception.DotSecurityException;
@@ -209,8 +209,11 @@ public User authorizePrincipal(final String username, final String passwd) throw
 			throw new DotSecurityException("The user was returned NULL");
 		}
 
-		// Validate password and rehash when is needed
-		if (!LoginFactory.passwordMatch(passwd, user) && !this.tryMatchJsonWebToken(passwd, user)) {
+		// Validate password and rehash when is needed. Refused while locked out; WebDAV attempts are
+		// counted apart from other logins from this address, so a client retrying a stale password does
+		// not lock its user out of the backend.
+		if (!LoginLockout.getInstance().passwordMatch(passwd, user, LoginLockout.webdavSource(),
+				this::tryMatchJsonWebToken)) {
 
 			Logger.debug(this, ()-> "The user's passwords didn't match");
 			throw new DotSecurityException("The user's passwords didn't match");

dotCMS/src/main/java/com/liferay/portal/ejb/UserManagerImpl.java

@@ -20,6 +20,7 @@
 package com.liferay.portal.ejb;
 
 import com.dotcms.api.system.user.UserService;
+import com.dotcms.auth.LoginLockout;
 import com.dotcms.business.CloseDBIfOpened;
 import com.dotcms.enterprise.AuthPipeProxy;
 import com.dotcms.enterprise.PasswordFactoryProxy;
@@ -41,6 +42,7 @@
 import com.dotmarketing.util.Config;
 import com.dotmarketing.util.EmailUtils;
 import com.dotmarketing.util.Logger;
+import com.dotmarketing.util.SecurityLogger;
 import com.dotmarketing.util.UtilMethods;
 import com.dotmarketing.util.WebKeys;
 import com.liferay.portal.NoSuchUserException;
@@ -547,6 +549,18 @@ private int authenticate(final String companyId, String login, final String pass
             return Authenticator.DNE;
         }
         this.checkPasswordExpiration(login, user);
+        // Counted before the password is checked, so parallel attempts get at most the limit between them
+        final LoginLockout.Attempt attempt = LoginLockout.getInstance().begin(user.getUserId(),
+                LoginLockout.clientAddress());
+        if (attempt.isRefused()) {
+            // Same work, delay, failure handlers and result as a wrong password, so a refused login gives
+            // no signal about whether the guess was right.
+            LoginFactory.passwordMatch(password, user);
+            SecurityLogger.logInfo(this.getClass(), "Login refused, locked out: user " + user.getUserId()
+                    + " from " + LoginLockout.clientAddress());
+            this.processFailedLogin(user, login, companyId, byEmailAddress, attempt, true);
+            return Authenticator.FAILURE;
+        }
         if (authResult == Authenticator.SUCCESS) {
             authResult = LoginFactory.passwordMatch(password, user) ? Authenticator.SUCCESS : Authenticator.FAILURE;
             Logger.debug(this, String.format("Does password for User '%s' match? %s", login,
@@ -564,10 +578,16 @@ private int authenticate(final String companyId, String login, final String pass
                     user.setFailedLoginAttempts(0);
                     UserUtil.update(user);
                 }
+                if (authResult == Authenticator.SUCCESS) {
+                    // Earlier attempts from this address no longer count toward a lock
+                    LoginLockout.getInstance().succeeded(attempt);
+                }
             }
         }
         if (authResult == Authenticator.FAILURE) {
-            this.processFailedLogin(user, login, companyId, byEmailAddress);
+            this.processFailedLogin(user, login, companyId, byEmailAddress, attempt, false);
+        } else if (authResult != Authenticator.SUCCESS) {
+            LoginLockout.getInstance().failed(attempt);
         }
         return authResult;
     }
@@ -695,13 +715,23 @@ private void checkUserStatus(final User user) throws UserActiveException {
      * @param companyId      The current Company ID.
      * @param byEmailAddress If the current authentication method is via email, set this to {@code true}. If it's
      *                       done via User ID, set to {@code false}.
+     * @param attempt        The login lockout attempt for this login; see {@link LoginLockout#begin(String, String)}.
+     * @param refused        If the login was refused by login lockout, set this to {@code true}: the request is
+     *                       delayed the same way, but the User's failure counter is not changed.
      */
     private void processFailedLogin(final User user, final String login, final String companyId,
-                                    final boolean byEmailAddress) {
+                                    final boolean byEmailAddress, final LoginLockout.Attempt attempt,
+                                    final boolean refused) {
         Logger.debug(this, String.format("Authentication for user '%s' has failed.", login));
+        // Recorded first, before the delay below, so the lock state is up to date as soon as possible
+        LoginLockout.getInstance().failed(attempt);
         try {
             this.runCustomOnFailureHandlers(companyId, login, byEmailAddress);
-            this.handleFailedLoginAttempt(user, login, companyId, byEmailAddress);
+            if (refused) {
+                this.delayFailedLogin(user, login);
+            } else {
+                this.handleFailedLoginAttempt(user, login, companyId, byEmailAddress);
+            }
         } catch (final Exception e) {
             final String errorMsg = String.format("An error occurred when handling failed login for User '%s': " +
                                                           "%s", login, e.getMessage());
@@ -747,27 +777,45 @@ private void runCustomOnFailureHandlers(String companyId, String login, boolean
      * @throws SystemException User failed attempts data could not be saved.
      * @throws AuthException   An error occurred when executing the handlers for maximum failed loin attempts.
      */
-    private void handleFailedLoginAttempt(final User user, final String login, final String companyId, final boolean byEmailAddress) throws SystemException, AuthException {
+    private void handleFailedLoginAttempt(final User user, final String login, final String companyId,
+                                          final boolean byEmailAddress) throws SystemException, AuthException {
         int failedLoginAttempts = user.getFailedLoginAttempts();
         Logger.debug(this, String.format("Current failed login attempts for user '%s' is %s", login,
                 failedLoginAttempts));
-        if (Config.getBooleanProperty(WebKeys.AUTH_FAILED_ATTEMPTS_DELAY_STRATEGY_ENABLED, true)) {
-            Logger.debug(this, String.format("Delaying login attempts for user '%s'", login));
-            this.delayRequest(failedLoginAttempts);
-        }
+        this.delayFailedLogin(user, login);
         user.setFailedLoginAttempts(++failedLoginAttempts);
         Logger.debug(this, String.format("Setting failed login attempts for user '%s' to %s", login,
                 user.getFailedLoginAttempts()));
         UserUtil.update(user);
         final int maxFailures = GetterUtil.get(PropsUtil.get(PropsUtil.AUTH_MAX_FAILURES_LIMIT), 0);
         Logger.debug(this, String.format("Maximum failed login attempts is %s", maxFailures));
+        final boolean lockoutEnabled = LoginLockout.getInstance().isEnabled(maxFailures);
         if ((failedLoginAttempts >= maxFailures) && (maxFailures != 0)) {
             Logger.debug(this, String.format("User '%s' failed to authenticate via '%s' after %s failed attempts",
                     login, byEmailAddress ? "email" : "user ID", failedLoginAttempts));
+            if (lockoutEnabled) {
+                // Locks are decided per address by LoginLockout. This account-wide count only paces the delay and
+                // the handlers below, so it starts over rather than growing with every failure from anywhere.
+                user.setFailedLoginAttempts(0);
+                UserUtil.update(user);
+            }
             this.runCustomMaxFailuresHandlers(login, companyId, byEmailAddress);
         }
     }
 
+    /**
+     * Applies the login-delaying strategy for a failed login, based on the User's current failed login count.
+     *
+     * @param user  The {@link User} that failed to log in.
+     * @param login The User's email or ID.
+     */
+    private void delayFailedLogin(final User user, final String login) {
+        if (Config.getBooleanProperty(WebKeys.AUTH_FAILED_ATTEMPTS_DELAY_STRATEGY_ENABLED, true)) {
+            Logger.debug(this, String.format("Delaying login attempts for user '%s'", login));
+            this.delayRequest(user.getFailedLoginAttempts());
+        }
+    }
+
     /**
      * Executes the list of custom failure handlers when the maximum number of login attempts has been reached. It can
      * be specified via the following property: {@link PropsUtil#AUTH_MAX_FAILURES}.
@@ -825,6 +873,7 @@ public void resetPassword(String userId, String token, String newPassword) throw
         try {
                 final User user = APILocator.getUserAPI().loadUserById(userId);
                 ResetPasswordTokenUtil.checkToken(user, token);
+                // Also clears any login lockout on the account.
                 APILocator.getUserAPI().updatePassword(user, newPassword, APILocator.getUserAPI().getSystemUser(), false);
         } catch (DotDataException e) {
             throw new IllegalArgumentException();

dotCMS/src/main/resources/postgres.sql

@@ -2472,6 +2472,18 @@ CREATE TABLE system_event_cursor (
     CONSTRAINT pk_system_event_cursor PRIMARY KEY (server_id)
 );
 
+-- Login attempt counts and locks per account and source (client address, 'webdav:' + address, or
+-- '*' for the account-wide count), shared by every node of a cluster.
+CREATE TABLE login_lockout (
+    user_id      VARCHAR(100) NOT NULL,
+    source       VARCHAR(100) NOT NULL,
+    failures     INT          NOT NULL,
+    window_start BIGINT       NOT NULL,
+    locked_until BIGINT       NOT NULL,
+    mod_date     TIMESTAMPTZ  NOT NULL,
+    CONSTRAINT pk_login_lockout PRIMARY KEY (user_id, source)
+);
+
 --Content Types improvement
 CREATE INDEX idx_lower_structure_name ON structure (LOWER(velocity_var_name));