Add account lockout after repeated failed logins

[mbiuki]

Closes #38066

Internal tracking: dotCMS/private-issues#777

Proposed Changes

• New com.dotcms.auth.LoginLockout, keyed on the account and the client address:

  • Per address: each address gets auth.max.failures.limit password attempts (default 5) within AUTH_LOCKOUT_WINDOW_SECONDS (default 3600). When they're used up without a success, that address is locked out of that account. Logins to the same account from other addresses are not affected.
  • WebDAV is counted under its own source (webdav:<address>), so a client retrying a stale password locks only WebDAV from that machine, not backend logins.
  • Account-wide limit: once failures from all addresses together reach AUTH_LOCKOUT_ACCOUNT_MAX_FAILURES within the window (default 100; 0 turns it off), the account is locked for every address.
  • Locks last a fixed AUTH_LOCKOUT_DURATION_SECONDS (default 900) and do not grow. A count that ages out of the window, or whose lock has run out, starts over.
  • AUTH_LOCKOUT_ENABLED=false turns locking off.

• Each attempt is counted before its password is checked, with a single INSERT ... RETURNING, and the returned count decides whether it may check a password. Parallel requests therefore get at most the limit between them.

• While a lock applies, the password is still checked, and the login fails with the same result as a wrong password. The auth.failure handlers and the failed-login delay run as for a wrong password, and nothing is counted.

• Every password check goes through it:

  • the main login flow (UserManagerImpl.authenticate: login form, REST basic auth)
  • WebDAV (DotWebdavHelper.authorizePrincipal)
  • the legacy LoginFactory.doLogin methods used by the publishing and maintenance ajax endpoints, including their LDAP branches

  API tokens accepted by WebDAV are not subject to the lock and are not counted.

• A successful login clears the count for its own address. A password reset, or a new password set by an administrator, clears all lockout state for the account and resets its failure count. Admin password changes cover UserAPI.updatePassword and a user save with a new password, as the Users portlet and REST user update do.

• The client address is HttpServletRequest.getRemoteAddr(). The bundled Tomcat RemoteIpValve takes X-Forwarded-For only from trusted proxies (private ranges by default). Installs behind other proxies need CMS_REMOTEIP_INTERNALPROXIES set; otherwise all logins share the proxy's address.

• State lives in a new login_lockout table, so all nodes share it: one row per account and source, plus one account-wide row.

• Schema comes from startup task Task261015CreateLoginLockoutTable (additive; postgres.sql updated). The task also:

  • restarts every user's failedloginattempts at 0, because dormant accounts can carry large values that set the delay on their next failed login
  • rebuilds the table if an earlier build of this branch created it with the old layout

Checklist

• Tests: LoginLockoutTest (in MainSuite1b), 21 cases, all passing locally against Postgres and OpenSearch. They cover:

  • per-address isolation, separate counts per address, and the account-wide limit
  • fixed lock length, and the count starting over after a lock runs out
  • attempts counted before the password check (20 parallel attempts, 5 allowed), and concurrent wrong logins
  • WebDAV locking only WebDAV, and an API token accepted while locked
  • both doLogin overloads
  • no counting while locked
  • reset-token, updatePassword and admin-save unlocks
  • a stale counter above the limit, and the upgrade task's reset
  • AUTH_LOCKOUT_ENABLED=false

  Not covered: the LDAP branch of doLogin. It only runs when auth.pipeline.pre names the LDAP authenticator, which LoginFactory reads into a static final.

• Translations

• Security Implications Contemplated

Release note

Logins now lock out after repeated failures. Five failed attempts from one address within an hour lock that address out of the account for 15 minutes. 100 failed attempts from all addresses within an hour lock the account for everyone for 15 minutes. This applies to backend login, REST basic auth, WebDAV (counted separately) and the legacy publishing endpoints. API tokens are not affected. Locks clear on their own, or when the password is reset or changed by an administrator. Set DOT_AUTH_LOCKOUT_ENABLED=false to turn lockout off. Behind a proxy, make sure Tomcat sees the real client address (CMS_REMOTEIP_INTERNALPROXIES).

Support runbook: a user is locked out

1. Try another network, or wait. Locks are per address and last 15 minutes, so the user can log in from another network or wait it out. Only the account-wide limit blocks every address.
2. Unlock now: reset the password through the "Forgot password" email, or have an administrator set a new password in Users. Either clears all lockout state for the account.
3. Unlock without changing the password: an administrator with database access can clear the state directly.
   • Everything for the user: DELETE FROM login_lockout WHERE user_id = '<user id>';
   • One address only: DELETE FROM login_lockout WHERE user_id = '<user id>' AND source = '<address>';
4. Diagnose: SELECT source, failures, to_timestamp(locked_until / 1000) FROM login_lockout WHERE user_id = '<user id>'; shows each address's count and lock. The security log records each lock with its address.
5. If every lock shows the same proxy or load-balancer address, Tomcat isn't seeing real client addresses; set CMS_REMOTEIP_INTERNALPROXIES.
6. To turn lockout off: set DOT_AUTH_LOCKOUT_ENABLED=false.

Additional Info

Open question: should lockout be on or off by default? The code ships with AUTH_LOCKOUT_ENABLED=true; reviewers please weigh in.

• Because locks are per account and source, failures from one address don't block the same account from elsewhere. On a shared address (office NAT, misconfigured proxy), only that one account is affected from that address.
• What remains: the account-wide limit locks an account for everyone once 100 failures arrive from several addresses within an hour.

Other behavior to review:

• user_.failedloginattempts is still incremented by the main login flow and still drives the failed-login delay and the auth.max.failures handlers. With lockout on, it starts over at the limit so the delay stays bounded; lock decisions come only from login_lockout.
• Every login now writes to login_lockout: the attempt is counted, then a success deletes the row. That is small next to the password hash each login already does.
• With lockout off, the legacy LoginFactory.doLogin paths no longer increment user_.failedloginattempts (they did before this PR); WebDAV never did.

🤖 Generated with Claude Code

https://claude.ai/code/session_0191fGSd1gBosfHHz1JCCHks

[semgrep-dotcms]

Semgrep found 11 CUSTOM_INJECTION-2 findings:

• dotCMS/src/main/java/com/dotcms/auth/LoginLockout.java
  • L323-329 - Triage
  • L323-328 - Triage
  • L323-327 - Triage
  • L323-326 - Triage
  • L323-326 - Triage
  • L323-326 - Triage
  • L323-325 - Triage
  • L323-325 - Triage
  • L323-325 - Triage
  • L323-324 - Triage
  • 1 more - Triage

🟡 Medium severity issue identified in your code:

The method identified is susceptible to injection. The input should be validated and properly
escaped.

If this is a critical or high severity finding, please also link this issue in the #security channel in Slack.

[fabrizzio-dotCMS]

Solid change overall. Keeping lockout on by default makes sense given the internal tracking issue. A few points before merge:

1. failedLoginAttempts increment is not atomic (likely bypass, not verified). handleFailedLoginAttempt and LoginLockout.recordFailure do read → +1 → save on the User. If N parallel attempts read the same value, they all write the same result, so N guesses count as one failure and the lock may never trigger. This race predates the PR, but the lockout now depends on it. A concurrent-requests test against one account would settle it; the fix would be an UPDATE ... SET failedlogins = failedlogins + 1 RETURNING failedlogins, deciding the lock on the returned value. Same root cause as the lockout_count open point, but with higher impact.

2. WebDAV failures now lock the whole account. test_webdav_failures_count_toward_lock shows this is intended, but a mounted drive with a stale password (Finder / Windows) retries on its own. It will lock the user out of the UI within seconds, re-lock on every expiry, and escalate to 24h. Please test with a real WebDAV client and confirm we accept that trade-off.

3. Accumulated counters lock on the first typo after upgrade. failedLoginAttempts only resets on success, so dormant accounts can already be above the limit. Suggest resetting failedlogins to 0 in Task261015CreateLoginLockoutTable.

4. Recovery path. Anyone who knows a user's email can keep that account locked with 5 requests per window. Password reset recovers the account, but it depends on working mail. Could we document a support runbook (admin sets a new password, or delete the login_lockout row) and add a release note saying lockout is on by default?

Minor: the counter reset means delayRequest never grows past the 4-failure delay, while every locked attempt still does a full hash. Fine as a trade-off, just worth stating. Fail-open on a login_lockout read error looks right to me.

Missing tests: WebDAV token accepted while locked, LDAP path refuses a locked account, pre-existing counter above the limit after upgrade, concurrent failures.

[mbiuki]

Thanks, this was useful. Fixes are in a6ac29b. Some points describe the version before b168bb4, which made locks per address and fixed at 15 minutes (no 24h growth), so I've answered against the current code.

1. Atomic counting. Lock decisions no longer use user_.failedloginattempts; they come from login_lockout. Your concern did apply there in another form: the lock was checked before the password and the failure counted after, so parallel requests could all pass the check. Each attempt is now counted before its password is checked, with one INSERT ... ON CONFLICT ... RETURNING failures, and the returned count decides whether it may check a password at all.

• test_attempts_are_counted_before_the_password_check starts 20 attempts at once; exactly 5 get to check a password.
• test_concurrent_wrong_logins_lock_the_address covers the main login flow end to end.

2. WebDAV. WebDAV is now counted under its own source (webdav:<address>). A drive retrying a stale password locks only WebDAV from that machine, for a fixed 15 minutes, and backend logins from it keep working (test_webdav_failures_lock_only_webdav). I haven't tried it with a real Finder or Windows mount.

3. Old counters. These no longer decide locks, but they do set the delay on the next failed login. The startup task now resets failedloginattempts to 0 (test_counter_above_limit_from_before_upgrade, which also runs the task).

4. Recovery. With locks per address, knowing an email isn't enough to lock the owner out. Only the account-wide limit (100 failures an hour, by default) locks the account for everyone, and that takes failures from several addresses, since each address is stopped after 5. The runbook and release note are in the PR description.

Minor: agreed. The delay stays bounded, and locked attempts still do the full hash on purpose, so timing doesn't show whether an account is locked.

Tests: I added the WebDAV token, stale counter and concurrency cases (21 tests, all passing locally). The LDAP branch isn't covered. It only runs when auth.pipeline.pre names the LDAP authenticator, and LoginFactory reads that into a static final, so a test can't switch it on.

[wezell]

I don't think we need a separate table for locking out. I think we just need a column in the user table called "lockout_date" that we write to and that we update when they are locked out. When they login successfully, we can delete the lockout date. Not sure what the extra table bring us?

[fabrizzio-dotCMS]

Follow-up on point 1: the separate login_lockout table is a good call. It's read straight from the DB on every login, so a lock or unlock is visible on all nodes immediately (as long as lock()/clear() aren't running inside a larger transaction).

The failure count is a different story: failedLoginAttempts still lives on the cached User. If attempts are spread across nodes, each node may be incrementing its own copy, which delays the lock. Not verified, but it's the same race as point 1, just across nodes, and the atomic SQL increment would fix both.

[fabrizzio-dotCMS]

I don't think we need a separate table for locking out. I think we just need a column in the user table called "lockout_date" that we write to and that we update when they are locked out. When they login successfully, we can delete the lockout date. Not sure what the extra table bring us?

The new table is not in cache. However, I don't see a cleanup operation anywhere in case of a user deletion

[mbiuki]

@wezell The table is there because the lock is no longer per account. Since b168bb4 it is per account and client address: each address gets its own count and its own lock, WebDAV is counted separately from other logins, and a looser account-wide count sits on top. That state is several rows per user (one per address that has failed recently, plus the account-wide one), so it doesn't fit in a single column on user_.

A single lockout_date on user_ would mean "this account is locked for everyone". Then failed logins from any address can keep a known user, including an administrator, out of their account, and keep doing so whenever the lock expires. The earlier review raised exactly that, and moving the lock to per address was the fix.

Other reasons for a separate table:

• Atomic counting. Each attempt is counted with one INSERT ... ON CONFLICT ... RETURNING before its password is checked, so parallel requests can't get past the limit. Doing the same on user_ would mean a write and a row lock on the user row for every login, on a table that Hibernate (UserPersistence) and the user cache also write.
• Short-lived rows. They are removed on success or when a new password is set, and they reset when their window runs out. That churn stays out of user_ and the user cache.
• Rollback safety. The new table is additive: a release rolled back onto this schema simply never reads it.

If we'd rather have the simpler per-account lock, a lockout_date column would work. But a known login name could then be used to keep that user out, which is the trade-off to decide. Happy to switch if that's the call.

[mbiuki]

@fabrizzio-dotCMS thanks.

Failure count on the cached User: failedLoginAttempts no longer takes part in lock decisions (since a6ac29b). Locks come only from login_lockout, which every node reads and writes in the database: each attempt is counted with one atomic INSERT ... ON CONFLICT ... RETURNING before its password is checked. The User count now only paces the failed-login delay and runs the auth.max.failures handlers, so a node-local copy lagging behind can't delay a lock.

On transactions: a password change clears the lock in the same transaction, which is what we want. If a login runs inside a wider transaction, its count holds the row lock until commit. Parallel attempts on the same row then wait their turn rather than skip ahead, so the limit still holds.

Cleanup: good catch, fixed in ef168f1.

• Deleting a user now deletes their login_lockout rows in the same transaction (UserFactoryImpl.delete).
• Whenever a lock is set, rows that no longer affect any login (lock run out and count older than the window) are purged for all accounts. Addresses that fail and never succeed no longer stay in the table for good.
• Tests: test_user_delete_removes_lockout_rows and test_expired_rows_are_purged_when_a_lock_is_set.